diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-09-14 15:12:56 +0000 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-09-14 15:12:56 +0000 |
| commit | c014b6b4e5c33180dd5cbff1dd1bb4623d1eca80 (patch) | |
| tree | 0aa4dbf0c9236ebd669b26240a18c08aef2cc50e /rubocop | |
| parent | 16daf112d6cfe2c87d8837382a00d88aa8c0ff5c (diff) | |
| download | gitlab-ce-c014b6b4e5c33180dd5cbff1dd1bb4623d1eca80.tar.gz | |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'rubocop')
128 files changed, 354 insertions, 399 deletions
diff --git a/rubocop/code_reuse_helpers.rb b/rubocop/code_reuse_helpers.rb index 87b907f8778..245bbc31cbd 100644 --- a/rubocop/code_reuse_helpers.rb +++ b/rubocop/code_reuse_helpers.rb @@ -163,7 +163,7 @@ module RuboCop each_send_node(node) do |send_node| next unless send_receiver_name_ends_with?(send_node, suffix) - add_offense(send_node, location: :expression, message: message) + add_offense(send_node, message: message) end end diff --git a/rubocop/cop/active_model_errors_direct_manipulation.rb b/rubocop/cop/active_model_errors_direct_manipulation.rb index e1e2840f37c..0a3af067743 100644 --- a/rubocop/cop/active_model_errors_direct_manipulation.rb +++ b/rubocop/cop/active_model_errors_direct_manipulation.rb @@ -6,7 +6,7 @@ module RuboCop # in preparation to upgrade to Rails 6.1 # # See https://gitlab.com/gitlab-org/gitlab/-/issues/225874 - class ActiveModelErrorsDirectManipulation < RuboCop::Cop::Cop + class ActiveModelErrorsDirectManipulation < RuboCop::Cop::Base MSG = 'Avoid manipulating errors hash directly. For more details check https://gitlab.com/gitlab-org/gitlab/-/issues/225874' MANIPULATIVE_METHODS = ":<< :append :clear :collect! :compact! :concat :delete :delete_at :delete_if :drop :drop_while :fill :filter! :keep_if :flatten! :insert :map! :pop :prepend :push :reject! :replace :reverse! :rotate! :select! :shift :shuffle! :slice! :sort! :sort_by! :uniq! :unshift" @@ -50,10 +50,10 @@ module RuboCop PATTERN def on_send(node) - add_offense(node, location: :expression) if active_model_errors_root_assignment?(node) - add_offense(node, location: :expression) if active_model_errors_root_manipulation?(node) - add_offense(node, location: :expression) if active_model_errors_manipulation?(node) - add_offense(node, location: :expression) if active_model_errors_assignment?(node) + add_offense(node) if active_model_errors_root_assignment?(node) + add_offense(node) if active_model_errors_root_manipulation?(node) + add_offense(node) if active_model_errors_manipulation?(node) + add_offense(node) if active_model_errors_assignment?(node) end end end diff --git a/rubocop/cop/active_record_association_reload.rb b/rubocop/cop/active_record_association_reload.rb index eb9fc8a0246..9a1e9674904 100644 --- a/rubocop/cop/active_record_association_reload.rb +++ b/rubocop/cop/active_record_association_reload.rb @@ -3,7 +3,7 @@ module RuboCop module Cop # Cop that blacklists the use of `reload`. - class ActiveRecordAssociationReload < RuboCop::Cop::Cop + class ActiveRecordAssociationReload < RuboCop::Cop::Base MSG = 'Use reset instead of reload. ' \ 'For more details check the https://gitlab.com/gitlab-org/gitlab-foss/issues/60218.' @@ -14,7 +14,7 @@ module RuboCop def on_send(node) return unless reload?(node) - add_offense(node, location: :selector) + add_offense(node.loc.selector) end end end diff --git a/rubocop/cop/api/base.rb b/rubocop/cop/api/base.rb index 85b19e9a833..c10115c8265 100644 --- a/rubocop/cop/api/base.rb +++ b/rubocop/cop/api/base.rb @@ -3,7 +3,9 @@ module RuboCop module Cop module API - class Base < RuboCop::Cop::Cop + class Base < RuboCop::Cop::Base + extend RuboCop::Cop::AutoCorrector + # This cop checks that APIs subclass API::Base. # # @example @@ -38,13 +40,9 @@ module RuboCop def on_class(node) grape_api_definition(node) do - add_offense(node.children[1]) - end - end - - def autocorrect(node) - lambda do |corrector| - corrector.replace(node, '::API::Base') + add_offense(node.children[1]) do |corrector| + corrector.replace(node.children[1], '::API::Base') + end end end end diff --git a/rubocop/cop/api/grape_array_missing_coerce.rb b/rubocop/cop/api/grape_array_missing_coerce.rb index 3d7a6a72d81..5cb80e113a4 100644 --- a/rubocop/cop/api/grape_array_missing_coerce.rb +++ b/rubocop/cop/api/grape_array_missing_coerce.rb @@ -3,7 +3,7 @@ module RuboCop module Cop module API - class GrapeArrayMissingCoerce < RuboCop::Cop::Cop + class GrapeArrayMissingCoerce < RuboCop::Cop::Base # This cop checks that Grape API parameters using an Array type # implement a coerce_with method: # diff --git a/rubocop/cop/avoid_becomes.rb b/rubocop/cop/avoid_becomes.rb index cfd6b3d2164..20df394c32c 100644 --- a/rubocop/cop/avoid_becomes.rb +++ b/rubocop/cop/avoid_becomes.rb @@ -9,7 +9,7 @@ module RuboCop # problems, even when a developer eager loaded all necessary associations. # # See https://gitlab.com/gitlab-org/gitlab/-/issues/23182 for more information. - class AvoidBecomes < RuboCop::Cop::Cop + class AvoidBecomes < RuboCop::Cop::Base MSG = 'Avoid the use of becomes(SomeConstant), as this creates a ' \ 'new object and throws away any eager loaded associations. ' \ 'When creating URLs in views, just use the path helpers directly. ' \ @@ -23,7 +23,7 @@ module RuboCop PATTERN def on_send(node) - add_offense(node, location: :expression) if becomes?(node) + add_offense(node) if becomes?(node) end end end diff --git a/rubocop/cop/avoid_break_from_strong_memoize.rb b/rubocop/cop/avoid_break_from_strong_memoize.rb index a3a8d0c3990..1a657a99966 100644 --- a/rubocop/cop/avoid_break_from_strong_memoize.rb +++ b/rubocop/cop/avoid_break_from_strong_memoize.rb @@ -20,7 +20,7 @@ module RuboCop # do_an_heavy_calculation # end # - class AvoidBreakFromStrongMemoize < RuboCop::Cop::Cop + class AvoidBreakFromStrongMemoize < RuboCop::Cop::Base MSG = 'Do not use break inside strong_memoize, use next instead.' def on_block(node) diff --git a/rubocop/cop/avoid_keyword_arguments_in_sidekiq_workers.rb b/rubocop/cop/avoid_keyword_arguments_in_sidekiq_workers.rb index da3cac073ad..ea7cdd5bf9d 100644 --- a/rubocop/cop/avoid_keyword_arguments_in_sidekiq_workers.rb +++ b/rubocop/cop/avoid_keyword_arguments_in_sidekiq_workers.rb @@ -3,7 +3,7 @@ module RuboCop module Cop # Cop that blacklists keyword arguments usage in Sidekiq workers - class AvoidKeywordArgumentsInSidekiqWorkers < RuboCop::Cop::Cop + class AvoidKeywordArgumentsInSidekiqWorkers < RuboCop::Cop::Base MSG = "Do not use keyword arguments in Sidekiq workers. " \ "For details, check https://github.com/mperham/sidekiq/issues/2372" OBSERVED_METHOD = :perform @@ -13,7 +13,7 @@ module RuboCop node.arguments.each do |argument| if argument.type == :kwarg || argument.type == :kwoptarg - add_offense(node, location: :expression) + add_offense(node) end end end diff --git a/rubocop/cop/avoid_return_from_blocks.rb b/rubocop/cop/avoid_return_from_blocks.rb index cc1868f10a4..61edfd0a789 100644 --- a/rubocop/cop/avoid_return_from_blocks.rb +++ b/rubocop/cop/avoid_return_from_blocks.rb @@ -20,7 +20,7 @@ module RuboCop # do_something_else # end # - class AvoidReturnFromBlocks < RuboCop::Cop::Cop + class AvoidReturnFromBlocks < RuboCop::Cop::Base MSG = 'Do not return from a block, use next or break instead.' DEF_METHODS = %i[define_method lambda].freeze WHITELISTED_METHODS = %i[each each_filename times loop].freeze diff --git a/rubocop/cop/avoid_route_redirect_leading_slash.rb b/rubocop/cop/avoid_route_redirect_leading_slash.rb index 0b0dc7d3d33..0535ae16a33 100644 --- a/rubocop/cop/avoid_route_redirect_leading_slash.rb +++ b/rubocop/cop/avoid_route_redirect_leading_slash.rb @@ -13,7 +13,9 @@ module RuboCop # root to: redirect('-/autocomplete/users') # - class AvoidRouteRedirectLeadingSlash < RuboCop::Cop::Cop + class AvoidRouteRedirectLeadingSlash < RuboCop::Cop::Base + extend RuboCop::Cop::AutoCorrector + MSG = 'Do not use a leading "/" in route redirects' def_node_matcher :leading_slash_in_redirect?, <<~PATTERN @@ -24,7 +26,9 @@ module RuboCop return unless in_routes?(node) return unless leading_slash_in_redirect?(node) - add_offense(node) + add_offense(node) do |corrector| + corrector.replace(node.loc.expression, remove_leading_slash(node)) + end end def has_leading_slash?(str) @@ -38,12 +42,6 @@ module RuboCop dirname.end_with?('config/routes') || filename.end_with?('routes.rb') end - def autocorrect(node) - lambda do |corrector| - corrector.replace(node.loc.expression, remove_leading_slash(node)) - end - end - def remove_leading_slash(node) node.source.sub('/', '') end diff --git a/rubocop/cop/ban_catch_throw.rb b/rubocop/cop/ban_catch_throw.rb index 42301d5512f..3f215eb3140 100644 --- a/rubocop/cop/ban_catch_throw.rb +++ b/rubocop/cop/ban_catch_throw.rb @@ -19,7 +19,7 @@ module RuboCop # # ... # end # - class BanCatchThrow < RuboCop::Cop::Cop + class BanCatchThrow < RuboCop::Cop::Base MSG = "Do not use catch or throw unless a gem's API demands it." def on_send(node) @@ -27,7 +27,7 @@ module RuboCop return unless receiver.nil? && %i[catch throw].include?(method_name) - add_offense(node, location: :expression) + add_offense(node) end end end diff --git a/rubocop/cop/code_reuse/finder.rb b/rubocop/cop/code_reuse/finder.rb index 1d70befe79b..ed008f613a4 100644 --- a/rubocop/cop/code_reuse/finder.rb +++ b/rubocop/cop/code_reuse/finder.rb @@ -6,7 +6,7 @@ module RuboCop module Cop module CodeReuse # Cop that enforces various code reuse rules for Finders. - class Finder < RuboCop::Cop::Cop + class Finder < RuboCop::Cop::Base include CodeReuseHelpers IN_FINDER = 'Finders can not be used inside a Finder.' diff --git a/rubocop/cop/code_reuse/presenter.rb b/rubocop/cop/code_reuse/presenter.rb index 6eef5e5a4b0..9ceb7bde54e 100644 --- a/rubocop/cop/code_reuse/presenter.rb +++ b/rubocop/cop/code_reuse/presenter.rb @@ -6,7 +6,7 @@ module RuboCop module Cop module CodeReuse # Cop that enforces various code reuse rules for Presenter classes. - class Presenter < RuboCop::Cop::Cop + class Presenter < RuboCop::Cop::Base include CodeReuseHelpers IN_SERVICE = 'Presenters can not be used in a Service class.' diff --git a/rubocop/cop/code_reuse/serializer.rb b/rubocop/cop/code_reuse/serializer.rb index 17a84ec31f7..493432c69da 100644 --- a/rubocop/cop/code_reuse/serializer.rb +++ b/rubocop/cop/code_reuse/serializer.rb @@ -6,7 +6,7 @@ module RuboCop module Cop module CodeReuse # Cop that enforces various code reuse rules for Serializer classes. - class Serializer < RuboCop::Cop::Cop + class Serializer < RuboCop::Cop::Base include CodeReuseHelpers IN_SERVICE = 'Serializers can not be used in a Service class.' diff --git a/rubocop/cop/code_reuse/service_class.rb b/rubocop/cop/code_reuse/service_class.rb index e403a87093c..e9002f232a2 100644 --- a/rubocop/cop/code_reuse/service_class.rb +++ b/rubocop/cop/code_reuse/service_class.rb @@ -6,7 +6,7 @@ module RuboCop module Cop module CodeReuse # Cop that enforces various code reuse rules for Service classes. - class ServiceClass < RuboCop::Cop::Cop + class ServiceClass < RuboCop::Cop::Base include CodeReuseHelpers IN_FINDER = 'Service classes can not be used in a Finder.' diff --git a/rubocop/cop/code_reuse/worker.rb b/rubocop/cop/code_reuse/worker.rb index 3a1120ac2a1..f39ffbe9c6f 100644 --- a/rubocop/cop/code_reuse/worker.rb +++ b/rubocop/cop/code_reuse/worker.rb @@ -6,7 +6,7 @@ module RuboCop module Cop module CodeReuse # Cop that enforces various code reuse rules for workers. - class Worker < RuboCop::Cop::Cop + class Worker < RuboCop::Cop::Base include CodeReuseHelpers IN_CONTROLLER = 'Workers can not be used in a controller.' diff --git a/rubocop/cop/database/disable_referential_integrity.rb b/rubocop/cop/database/disable_referential_integrity.rb index 80d52678011..9e201e2d143 100644 --- a/rubocop/cop/database/disable_referential_integrity.rb +++ b/rubocop/cop/database/disable_referential_integrity.rb @@ -4,7 +4,7 @@ module RuboCop module Cop module Database # Cop that checks if 'disable_referential_integrity' method is called. - class DisableReferentialIntegrity < RuboCop::Cop::Cop + class DisableReferentialIntegrity < RuboCop::Cop::Base MSG = <<~TEXT Do not use `disable_referential_integrity`, disable triggers in a safe transaction instead. Follow the format: diff --git a/rubocop/cop/database/establish_connection.rb b/rubocop/cop/database/establish_connection.rb index 20454887ce2..3681974640b 100644 --- a/rubocop/cop/database/establish_connection.rb +++ b/rubocop/cop/database/establish_connection.rb @@ -3,7 +3,7 @@ module RuboCop module Cop module Database - class EstablishConnection < RuboCop::Cop::Cop + class EstablishConnection < RuboCop::Cop::Base MSG = "Don't establish new database connections, as this slows down " \ 'tests and may result in new connections using an incorrect configuration' @@ -12,7 +12,7 @@ module RuboCop PATTERN def on_send(node) - add_offense(node, location: :expression) if establish_connection?(node) + add_offense(node) if establish_connection?(node) end end end diff --git a/rubocop/cop/database/multiple_databases.rb b/rubocop/cop/database/multiple_databases.rb index 1ac9bb4473b..33ff8acd4d8 100644 --- a/rubocop/cop/database/multiple_databases.rb +++ b/rubocop/cop/database/multiple_databases.rb @@ -10,7 +10,7 @@ module RuboCop # # good # ApplicationRecord.connection # - class MultipleDatabases < RuboCop::Cop::Cop + class MultipleDatabases < RuboCop::Cop::Base AR_BASE_MESSAGE = <<~EOF Do not use methods from ActiveRecord::Base, use the ApplicationRecord class instead For fixing offenses related to the ActiveRecord::Base.transaction method, see our guidelines: @@ -32,7 +32,7 @@ module RuboCop active_record_base_method = node.children[1] return if method_is_allowed?(active_record_base_method) - add_offense(node, location: :expression, message: AR_BASE_MESSAGE) + add_offense(node, message: AR_BASE_MESSAGE) end private diff --git a/rubocop/cop/database/rescue_query_canceled.rb b/rubocop/cop/database/rescue_query_canceled.rb index 1238f9ed911..01f3ba272c2 100644 --- a/rubocop/cop/database/rescue_query_canceled.rb +++ b/rubocop/cop/database/rescue_query_canceled.rb @@ -20,7 +20,7 @@ module RuboCop # # good # # run_cheap_queries_with_each_batch - class RescueQueryCanceled < RuboCop::Cop::Cop + class RescueQueryCanceled < RuboCop::Cop::Base MSG = <<~EOF Avoid rescuing the `ActiveRecord::QueryCanceled` class. diff --git a/rubocop/cop/database/rescue_statement_timeout.rb b/rubocop/cop/database/rescue_statement_timeout.rb index 0b75a441e29..a0e9396e8f4 100644 --- a/rubocop/cop/database/rescue_statement_timeout.rb +++ b/rubocop/cop/database/rescue_statement_timeout.rb @@ -20,7 +20,7 @@ module RuboCop # # good # # run_cheap_queries_with_each_batch - class RescueStatementTimeout < RuboCop::Cop::Cop + class RescueStatementTimeout < RuboCop::Cop::Base MSG = <<~EOF Avoid rescuing the `ActiveRecord::StatementTimeout` class. diff --git a/rubocop/cop/default_scope.rb b/rubocop/cop/default_scope.rb index 39f8c8e9ed0..930a69be881 100644 --- a/rubocop/cop/default_scope.rb +++ b/rubocop/cop/default_scope.rb @@ -3,7 +3,7 @@ module RuboCop module Cop # Cop that blacklists the use of `default_scope`. - class DefaultScope < RuboCop::Cop::Cop + class DefaultScope < RuboCop::Cop::Base MSG = <<~EOF Do not use `default_scope`, as it does not follow the principle of least surprise. See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/33847 @@ -17,7 +17,7 @@ module RuboCop def on_send(node) return unless default_scope?(node) - add_offense(node, location: :expression) + add_offense(node) end end end diff --git a/rubocop/cop/destroy_all.rb b/rubocop/cop/destroy_all.rb index 38b6cb40f91..78e5d0f25f3 100644 --- a/rubocop/cop/destroy_all.rb +++ b/rubocop/cop/destroy_all.rb @@ -3,7 +3,7 @@ module RuboCop module Cop # Cop that blacklists the use of `destroy_all`. - class DestroyAll < RuboCop::Cop::Cop + class DestroyAll < RuboCop::Cop::Base MSG = 'Use `delete_all` instead of `destroy_all`. ' \ '`destroy_all` will load the rows into memory, then execute a ' \ '`DELETE` for every individual row.' @@ -15,7 +15,7 @@ module RuboCop def on_send(node) return unless destroy_all?(node) - add_offense(node, location: :expression) + add_offense(node) end end end diff --git a/rubocop/cop/file_decompression.rb b/rubocop/cop/file_decompression.rb index 44813244028..e1e645e6d62 100644 --- a/rubocop/cop/file_decompression.rb +++ b/rubocop/cop/file_decompression.rb @@ -3,7 +3,7 @@ module RuboCop module Cop # Check for symlinks when extracting files to avoid arbitrary file reading. - class FileDecompression < RuboCop::Cop::Cop + class FileDecompression < RuboCop::Cop::Base MSG = <<~EOF While extracting files check for symlink to avoid arbitrary file reading. https://gitlab.com/gitlab-com/gl-infra/production/-/issues/6132 @@ -29,7 +29,7 @@ module RuboCop def on_send(node) system?(node) do |match| - add_offense(node, location: :expression, message: MSG) if forbidden_command?(match) + add_offense(node, message: MSG) if forbidden_command?(match) end end diff --git a/rubocop/cop/filename_length.rb b/rubocop/cop/filename_length.rb index 815db7de3f5..948f69cc88c 100644 --- a/rubocop/cop/filename_length.rb +++ b/rubocop/cop/filename_length.rb @@ -2,7 +2,7 @@ module RuboCop module Cop - class FilenameLength < Cop + class FilenameLength < RuboCop::Cop::Base include RangeHelp FILEPATH_MAX_BYTES = 256 @@ -10,14 +10,14 @@ module RuboCop 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" - def investigate(processed_source) + def on_new_investigation file_path = processed_source.file_path return if config.file_to_exclude?(file_path) if file_path.bytesize > FILEPATH_MAX_BYTES - add_offense(nil, location: source_range(processed_source.buffer, 1, 0, 1), message: MSG_FILEPATH_LEN) + add_offense(source_range(processed_source.buffer, 1, 0), message: MSG_FILEPATH_LEN) elsif File.basename(file_path).bytesize > FILENAME_MAX_BYTES - add_offense(nil, location: source_range(processed_source.buffer, 1, 0, 1), message: MSG_FILENAME_LEN) + add_offense(source_range(processed_source.buffer, 1, 0), message: MSG_FILENAME_LEN) end end end diff --git a/rubocop/cop/gitlab/avoid_feature_category_not_owned.rb b/rubocop/cop/gitlab/avoid_feature_category_not_owned.rb index fb790f44a96..05d7824f92e 100644 --- a/rubocop/cop/gitlab/avoid_feature_category_not_owned.rb +++ b/rubocop/cop/gitlab/avoid_feature_category_not_owned.rb @@ -5,7 +5,7 @@ require_relative '../../code_reuse_helpers' module RuboCop module Cop module Gitlab - class AvoidFeatureCategoryNotOwned < RuboCop::Cop::Cop + class AvoidFeatureCategoryNotOwned < RuboCop::Cop::Base include ::RuboCop::CodeReuseHelpers MSG = 'Avoid adding new endpoints with `feature_category :not_owned`. See https://docs.gitlab.com/ee/development/feature_categorization' @@ -25,7 +25,7 @@ module RuboCop return unless file_needs_feature_category?(node) return unless setting_not_owned?(node) - add_offense(node, location: :expression) + add_offense(node) end private diff --git a/rubocop/cop/gitlab/avoid_feature_get.rb b/rubocop/cop/gitlab/avoid_feature_get.rb index 7664f66521a..68aaff8aeff 100644 --- a/rubocop/cop/gitlab/avoid_feature_get.rb +++ b/rubocop/cop/gitlab/avoid_feature_get.rb @@ -20,7 +20,7 @@ module RuboCop # Feature.enable_percentage_of_time(:x, 100) # Feature.remove(:x) # - class AvoidFeatureGet < RuboCop::Cop::Cop + class AvoidFeatureGet < RuboCop::Cop::Base MSG = 'Use `stub_feature_flags` method instead of `Feature.get`. ' \ 'See doc/development/feature_flags/index.md#feature-flags-in-tests for more information.' @@ -31,7 +31,7 @@ module RuboCop def on_send(node) return unless feature_get?(node) - add_offense(node, location: :selector) + add_offense(node.loc.selector) end end end diff --git a/rubocop/cop/gitlab/avoid_uploaded_file_from_params.rb b/rubocop/cop/gitlab/avoid_uploaded_file_from_params.rb index 0795f96732d..cb3382c1b75 100644 --- a/rubocop/cop/gitlab/avoid_uploaded_file_from_params.rb +++ b/rubocop/cop/gitlab/avoid_uploaded_file_from_params.rb @@ -33,7 +33,7 @@ module RuboCop # uploaded_file = declared_params[:file] # end # end - class AvoidUploadedFileFromParams < RuboCop::Cop::Cop + class AvoidUploadedFileFromParams < RuboCop::Cop::Base MSG = 'Use the `UploadedFile` set by `multipart.rb` instead of calling `UploadedFile.from_params` directly. See https://docs.gitlab.com/ee/development/uploads/working_with_uploads.html' def_node_matcher :calling_uploaded_file_from_params?, <<~PATTERN @@ -43,7 +43,7 @@ module RuboCop def on_send(node) return unless calling_uploaded_file_from_params?(node) - add_offense(node, location: :expression) + add_offense(node) end end end diff --git a/rubocop/cop/gitlab/bulk_insert.rb b/rubocop/cop/gitlab/bulk_insert.rb index baaefc2533c..f88dcde2dc2 100644 --- a/rubocop/cop/gitlab/bulk_insert.rb +++ b/rubocop/cop/gitlab/bulk_insert.rb @@ -5,7 +5,7 @@ module RuboCop module Gitlab # Cop that disallows the use of `legacy_bulk_insert`, in favour of using # the `BulkInsertSafe` module. - class BulkInsert < RuboCop::Cop::Cop + class BulkInsert < RuboCop::Cop::Base MSG = 'Use the `BulkInsertSafe` concern, instead of using `LegacyBulkInsert.bulk_insert`. See https://docs.gitlab.com/ee/development/insert_into_tables_in_batches.html' def_node_matcher :raw_union?, <<~PATTERN @@ -15,7 +15,7 @@ module RuboCop def on_send(node) return unless raw_union?(node) - add_offense(node, location: :expression) + add_offense(node) end end end diff --git a/rubocop/cop/gitlab/change_timezone.rb b/rubocop/cop/gitlab/change_timezone.rb index c30a057d51c..a5d9393b6c3 100644 --- a/rubocop/cop/gitlab/change_timezone.rb +++ b/rubocop/cop/gitlab/change_timezone.rb @@ -3,7 +3,7 @@ module RuboCop module Cop module Gitlab - class ChangeTimezone < RuboCop::Cop::Cop + class ChangeTimezone < RuboCop::Cop::Base MSG = "Do not change timezone in the runtime (application or rspec), " \ "it could result in silently modifying other behavior." diff --git a/rubocop/cop/gitlab/const_get_inherit_false.rb b/rubocop/cop/gitlab/const_get_inherit_false.rb index 3d3bbc4c8d3..e44097a8d9e 100644 --- a/rubocop/cop/gitlab/const_get_inherit_false.rb +++ b/rubocop/cop/gitlab/const_get_inherit_false.rb @@ -6,7 +6,9 @@ module RuboCop # Cop that encourages usage of inherit=false for 2nd argument when using const_get. # # See https://gitlab.com/gitlab-org/gitlab/issues/27678 - class ConstGetInheritFalse < RuboCop::Cop::Cop + class ConstGetInheritFalse < RuboCop::Cop::Base + extend RuboCop::Cop::AutoCorrector + MSG = 'Use inherit=false when using const_get.' def_node_matcher :const_get?, <<~PATTERN @@ -17,11 +19,7 @@ module RuboCop return unless const_get?(node) return if second_argument(node)&.false_type? - add_offense(node, location: :selector) - end - - def autocorrect(node) - lambda do |corrector| + add_offense(node.loc.selector) do |corrector| if arg = second_argument(node) corrector.replace(arg.source_range, 'false') else diff --git a/rubocop/cop/gitlab/delegate_predicate_methods.rb b/rubocop/cop/gitlab/delegate_predicate_methods.rb index 43b5184faab..9e6a2823719 100644 --- a/rubocop/cop/gitlab/delegate_predicate_methods.rb +++ b/rubocop/cop/gitlab/delegate_predicate_methods.rb @@ -21,7 +21,7 @@ module RuboCop # def is_foo? # !!bar&.is_foo? # end - class DelegatePredicateMethods < RuboCop::Cop::Cop + class DelegatePredicateMethods < RuboCop::Cop::Base 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 diff --git a/rubocop/cop/gitlab/deprecate_track_redis_hll_event.rb b/rubocop/cop/gitlab/deprecate_track_redis_hll_event.rb index 3e30f3aa4d0..58411357eeb 100644 --- a/rubocop/cop/gitlab/deprecate_track_redis_hll_event.rb +++ b/rubocop/cop/gitlab/deprecate_track_redis_hll_event.rb @@ -14,7 +14,7 @@ module RuboCop # # # good # track_event :show, name: 'g_analytics_valuestream', destinations: [:redis_hll] - class DeprecateTrackRedisHLLEvent < RuboCop::Cop::Cop + 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' @@ -25,7 +25,7 @@ module RuboCop def on_send(node) return unless track_redis_hll_event_used?(node) - add_offense(node, location: :selector) + add_offense(node.loc.selector) end end end diff --git a/rubocop/cop/gitlab/event_store_subscriber.rb b/rubocop/cop/gitlab/event_store_subscriber.rb index 0b2dd94bc42..7e4cc3e66cc 100644 --- a/rubocop/cop/gitlab/event_store_subscriber.rb +++ b/rubocop/cop/gitlab/event_store_subscriber.rb @@ -30,7 +30,7 @@ module RuboCop # end # end # - class EventStoreSubscriber < RuboCop::Cop::Cop + 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)`." diff --git a/rubocop/cop/gitlab/except.rb b/rubocop/cop/gitlab/except.rb index 24da6962457..d20cf47b473 100644 --- a/rubocop/cop/gitlab/except.rb +++ b/rubocop/cop/gitlab/except.rb @@ -5,7 +5,7 @@ module RuboCop module Gitlab # Cop that disallows the use of `Gitlab::SQL::Except`, in favour of using # the `FromExcept` module. - class Except < RuboCop::Cop::Cop + class Except < RuboCop::Cop::Base MSG = 'Use the `FromExcept` concern, instead of using `Gitlab::SQL::Except` directly' def_node_matcher :raw_except?, <<~PATTERN @@ -15,7 +15,7 @@ module RuboCop def on_send(node) return unless raw_except?(node) - add_offense(node, location: :expression) + add_offense(node) end end end diff --git a/rubocop/cop/gitlab/feature_available_usage.rb b/rubocop/cop/gitlab/feature_available_usage.rb index f748b7d9111..3e0385c4018 100644 --- a/rubocop/cop/gitlab/feature_available_usage.rb +++ b/rubocop/cop/gitlab/feature_available_usage.rb @@ -4,7 +4,7 @@ module RuboCop module Cop module Gitlab # Cop that checks for correct calling of #feature_available? - class FeatureAvailableUsage < RuboCop::Cop::Cop + class FeatureAvailableUsage < RuboCop::Cop::Base 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." @@ -38,9 +38,9 @@ module RuboCop 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)) + add_offense(node, message: licensed_feature_message(node)) elsif args_count(node) < 2 - add_offense(node, location: :expression, message: NOT_ENOUGH_ARGS_MSG) + add_offense(node, message: NOT_ENOUGH_ARGS_MSG) end end diff --git a/rubocop/cop/gitlab/finder_with_find_by.rb b/rubocop/cop/gitlab/finder_with_find_by.rb index 8fa9fe4a2f9..ac454398f9c 100644 --- a/rubocop/cop/gitlab/finder_with_find_by.rb +++ b/rubocop/cop/gitlab/finder_with_find_by.rb @@ -3,8 +3,10 @@ module RuboCop module Cop module Gitlab - class FinderWithFindBy < RuboCop::Cop::Cop - FIND_PATTERN = /\Afind(_by\!?)?\z/.freeze + class FinderWithFindBy < RuboCop::Cop::Base + extend RuboCop::Cop::AutoCorrector + + FIND_PATTERN = /\Afind(_by!?)?\z/.freeze ALLOWED_MODULES = ['FinderMethods'].freeze def message(used_method) @@ -18,13 +20,9 @@ module RuboCop end def on_send(node) - if find_on_execute?(node) && !allowed_module?(node) - add_offense(node, location: :selector, message: message(node.method_name)) - end - end + return unless find_on_execute?(node) && !allowed_module?(node) - def autocorrect(node) - lambda do |corrector| + add_offense(node.loc.selector, message: message(node.method_name)) do |corrector| upto_including_execute = node.descendants.first.source_range before_execute = node.descendants[1].source_range range_to_remove = node.source_range diff --git a/rubocop/cop/gitlab/httparty.rb b/rubocop/cop/gitlab/httparty.rb index 20f0c381e11..f57c605ce91 100644 --- a/rubocop/cop/gitlab/httparty.rb +++ b/rubocop/cop/gitlab/httparty.rb @@ -3,7 +3,9 @@ module RuboCop module Cop module Gitlab - class HTTParty < RuboCop::Cop::Cop + class HTTParty < RuboCop::Cop::Base + extend RuboCop::Cop::AutoCorrector + MSG_SEND = <<~EOL Avoid calling `HTTParty` directly. Instead, use the Gitlab::HTTP wrapper. To allow request to localhost or the private network set @@ -25,31 +27,18 @@ module RuboCop PATTERN def on_send(node) - add_offense(node, location: :expression, message: MSG_SEND) if httparty_node?(node) - add_offense(node, location: :expression, message: MSG_INCLUDE) if includes_httparty?(node) - end - - def autocorrect(node) - if includes_httparty?(node) - autocorrect_includes_httparty(node) - else - autocorrect_httparty_node(node) - end - end - - def autocorrect_includes_httparty(node) - lambda do |corrector| - corrector.remove(node.source_range) - end - end - - def autocorrect_httparty_node(node) - _, method_name, *arg_nodes = *node - - replacement = "Gitlab::HTTP.#{method_name}(#{arg_nodes.map(&:source).join(', ')})" - - lambda do |corrector| - corrector.replace(node.source_range, replacement) + if httparty_node?(node) + add_offense(node, message: MSG_SEND) do |corrector| + _, method_name, *arg_nodes = *node + + replacement = "Gitlab::HTTP.#{method_name}(#{arg_nodes.map(&:source).join(', ')})" + + corrector.replace(node.source_range, replacement) + end + elsif includes_httparty?(node) + add_offense(node, message: MSG_INCLUDE) do |corrector| + corrector.remove(node.source_range) + end end end end diff --git a/rubocop/cop/gitlab/intersect.rb b/rubocop/cop/gitlab/intersect.rb index 4b61073b804..e608b25cbe1 100644 --- a/rubocop/cop/gitlab/intersect.rb +++ b/rubocop/cop/gitlab/intersect.rb @@ -5,7 +5,7 @@ module RuboCop module Gitlab # Cop that disallows the use of `Gitlab::SQL::Intersect`, in favour of using # the `FromIntersect` module. - class Intersect < RuboCop::Cop::Cop + class Intersect < RuboCop::Cop::Base MSG = 'Use the `FromIntersect` concern, instead of using `Gitlab::SQL::Intersect` directly' def_node_matcher :raw_intersect?, <<~PATTERN @@ -15,7 +15,7 @@ module RuboCop def on_send(node) return unless raw_intersect?(node) - add_offense(node, location: :expression) + add_offense(node) end end end diff --git a/rubocop/cop/gitlab/json.rb b/rubocop/cop/gitlab/json.rb index d2ba0012ca0..56846e3c276 100644 --- a/rubocop/cop/gitlab/json.rb +++ b/rubocop/cop/gitlab/json.rb @@ -3,7 +3,9 @@ module RuboCop module Cop module Gitlab - class Json < RuboCop::Cop::Cop + class Json < RuboCop::Cop::Base + extend RuboCop::Cop::AutoCorrector + MSG = <<~EOL Avoid calling `JSON` directly. Instead, use the `Gitlab::Json` wrapper. This allows us to alter the JSON parser being used. @@ -14,19 +16,13 @@ module RuboCop PATTERN def on_send(node) - add_offense(node) if json_node?(node) - end - - def autocorrect(node) - autocorrect_json_node(node) - end + return unless json_node?(node) - def autocorrect_json_node(node) - _, method_name, *arg_nodes = *node + add_offense(node) do |corrector| + _, method_name, *arg_nodes = *node - replacement = "Gitlab::Json.#{method_name}(#{arg_nodes.map(&:source).join(', ')})" + replacement = "Gitlab::Json.#{method_name}(#{arg_nodes.map(&:source).join(', ')})" - lambda do |corrector| corrector.replace(node.source_range, replacement) end end diff --git a/rubocop/cop/gitlab/mark_used_feature_flags.rb b/rubocop/cop/gitlab/mark_used_feature_flags.rb index 8fbcc56b906..8d8c84e78f5 100644 --- a/rubocop/cop/gitlab/mark_used_feature_flags.rb +++ b/rubocop/cop/gitlab/mark_used_feature_flags.rb @@ -9,7 +9,7 @@ module RuboCop # # The files set in `tmp/feature_flags/*.used` can then be used for verification purpose. # - class MarkUsedFeatureFlags < RuboCop::Cop::Cop + class MarkUsedFeatureFlags < RuboCop::Cop::Base include RuboCop::CodeReuseHelpers FEATURE_METHODS = %i[enabled? disabled?].freeze diff --git a/rubocop/cop/gitlab/module_with_instance_variables.rb b/rubocop/cop/gitlab/module_with_instance_variables.rb index 40cdc0d3a57..e43ede61b7e 100644 --- a/rubocop/cop/gitlab/module_with_instance_variables.rb +++ b/rubocop/cop/gitlab/module_with_instance_variables.rb @@ -3,7 +3,7 @@ module RuboCop module Cop module Gitlab - class ModuleWithInstanceVariables < RuboCop::Cop::Cop + class ModuleWithInstanceVariables < RuboCop::Cop::Base MSG = <<~EOL Do not use instance variables in a module. Please read this for the rationale behind it: @@ -32,12 +32,12 @@ module RuboCop if only_ivar_or_assignment?(definition) # We don't allow if any other ivar is used definition.each_descendant(:ivar) do |offense| - add_offense(offense, location: :expression) + add_offense(offense) end # We allow initialize method and single ivar elsif !initialize_method?(definition) && !single_ivar?(definition) definition.each_descendant(:ivar, :ivasgn) do |offense| - add_offense(offense, location: :expression) + add_offense(offense) end end end diff --git a/rubocop/cop/gitlab/policy_rule_boolean.rb b/rubocop/cop/gitlab/policy_rule_boolean.rb index ca69eebab6e..ddf100fc8a1 100644 --- a/rubocop/cop/gitlab/policy_rule_boolean.rb +++ b/rubocop/cop/gitlab/policy_rule_boolean.rb @@ -22,7 +22,7 @@ module RuboCop # # good # rule { conducts_electricity & can?(:magnetize) }.enable :motor # rule { ~conducts_electricity & batteries }.enable :motor - class PolicyRuleBoolean < RuboCop::Cop::Cop + class PolicyRuleBoolean < RuboCop::Cop::Base def_node_search :has_and_operator?, <<~PATTERN (and ...) PATTERN diff --git a/rubocop/cop/gitlab/predicate_memoization.rb b/rubocop/cop/gitlab/predicate_memoization.rb index 4c851f90238..3fbd004655f 100644 --- a/rubocop/cop/gitlab/predicate_memoization.rb +++ b/rubocop/cop/gitlab/predicate_memoization.rb @@ -3,7 +3,7 @@ module RuboCop module Cop module Gitlab - class PredicateMemoization < RuboCop::Cop::Cop + class PredicateMemoization < RuboCop::Cop::Base MSG = <<~EOL Avoid using `@value ||= query` inside predicate methods in order to properly memoize `false` or `nil` values. diff --git a/rubocop/cop/gitlab/rails_logger.rb b/rubocop/cop/gitlab/rails_logger.rb index 5a1695ce56e..ae05491ca7d 100644 --- a/rubocop/cop/gitlab/rails_logger.rb +++ b/rubocop/cop/gitlab/rails_logger.rb @@ -38,7 +38,7 @@ module RuboCop def on_send(node) return unless rails_logger_log?(node) - add_offense(node, location: :expression) + add_offense(node) end end end diff --git a/rubocop/cop/gitlab/union.rb b/rubocop/cop/gitlab/union.rb index c44c847657b..36ec6bb9b7f 100644 --- a/rubocop/cop/gitlab/union.rb +++ b/rubocop/cop/gitlab/union.rb @@ -5,7 +5,7 @@ module RuboCop module Gitlab # Cop that disallows the use of `Gitlab::SQL::Union`, in favour of using # the `FromUnion` module. - class Union < RuboCop::Cop::Cop + class Union < RuboCop::Cop::Base MSG = 'Use the `FromUnion` concern, instead of using `Gitlab::SQL::Union` directly' def_node_matcher :raw_union?, <<~PATTERN @@ -15,7 +15,7 @@ module RuboCop def on_send(node) return unless raw_union?(node) - add_offense(node, location: :expression) + add_offense(node) end end end diff --git a/rubocop/cop/graphql/authorize_types.rb b/rubocop/cop/graphql/authorize_types.rb index c96919343d6..7bd2cd9f7ef 100644 --- a/rubocop/cop/graphql/authorize_types.rb +++ b/rubocop/cop/graphql/authorize_types.rb @@ -3,7 +3,7 @@ module RuboCop module Cop module Graphql - class AuthorizeTypes < RuboCop::Cop::Cop + class AuthorizeTypes < RuboCop::Cop::Base MSG = 'Add an `authorize :ability` call to the type: '\ 'https://docs.gitlab.com/ee/development/graphql_guide/authorization.html#type-authorization' @@ -19,7 +19,7 @@ module RuboCop return if allowed?(class_constant(node)) return if allowed?(superclass_constant(node)) - add_offense(node, location: :expression) unless authorize?(node) + add_offense(node) unless authorize?(node) end private diff --git a/rubocop/cop/graphql/descriptions.rb b/rubocop/cop/graphql/descriptions.rb index 0d69fd55931..3c945507699 100644 --- a/rubocop/cop/graphql/descriptions.rb +++ b/rubocop/cop/graphql/descriptions.rb @@ -42,7 +42,9 @@ module RuboCop module Cop module Graphql - class Descriptions < RuboCop::Cop::Cop + class Descriptions < RuboCop::Cop::Base + 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}" @@ -74,15 +76,17 @@ module RuboCop description = locate_description(node) - return add_offense(node, location: :expression, message: MSG_NO_DESCRIPTION) unless description + message = if description.nil? + MSG_NO_DESCRIPTION + elsif no_period?(description) + MSG_NO_PERIOD + elsif bad_start?(description) + MSG_BAD_START + end - add_offense(node, location: :expression, message: MSG_NO_PERIOD) if no_period?(description) - add_offense(node, location: :expression, message: MSG_BAD_START) if bad_start?(description) - end + return unless message - # Autocorrect missing periods at end of description. - def autocorrect(node) - lambda do |corrector| + add_offense(node, message: message) do |corrector| description = locate_description(node) next unless description diff --git a/rubocop/cop/graphql/gid_expected_type.rb b/rubocop/cop/graphql/gid_expected_type.rb index 354c5516752..7e802e6d2db 100644 --- a/rubocop/cop/graphql/gid_expected_type.rb +++ b/rubocop/cop/graphql/gid_expected_type.rb @@ -3,7 +3,7 @@ module RuboCop module Cop module Graphql - class GIDExpectedType < RuboCop::Cop::Cop + class GIDExpectedType < RuboCop::Cop::Base MSG = 'Add an expected_type parameter to #object_from_id calls if possible.' def_node_search :id_from_object?, <<~PATTERN diff --git a/rubocop/cop/graphql/graphql_name_position.rb b/rubocop/cop/graphql/graphql_name_position.rb index f18d65588cc..b876fb5b088 100644 --- a/rubocop/cop/graphql/graphql_name_position.rb +++ b/rubocop/cop/graphql/graphql_name_position.rb @@ -20,7 +20,7 @@ module RuboCop module Cop module Graphql - class GraphqlNamePosition < RuboCop::Cop::Cop + class GraphqlNamePosition < RuboCop::Cop::Base MSG = '`graphql_name` should be the first line of the class: '\ 'https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#naming-conventions' @@ -32,7 +32,7 @@ module RuboCop return unless graphql_name?(node) return if node.body.single_line? - add_offense(node, location: :expression) unless graphql_name?(node.body.children.first) + add_offense(node) unless graphql_name?(node.body.children.first) end end end diff --git a/rubocop/cop/graphql/id_type.rb b/rubocop/cop/graphql/id_type.rb index ba973242efa..eb677aa4507 100644 --- a/rubocop/cop/graphql/id_type.rb +++ b/rubocop/cop/graphql/id_type.rb @@ -3,7 +3,7 @@ module RuboCop module Cop module Graphql - class IDType < RuboCop::Cop::Cop + class IDType < RuboCop::Cop::Base MSG = 'Do not use GraphQL::Types::ID, use a specific GlobalIDType instead' WHITELISTED_ARGUMENTS = %i[iid full_path project_path group_path target_project_path namespace_path].freeze diff --git a/rubocop/cop/graphql/json_type.rb b/rubocop/cop/graphql/json_type.rb index 8518a771cbd..2525dc427b8 100644 --- a/rubocop/cop/graphql/json_type.rb +++ b/rubocop/cop/graphql/json_type.rb @@ -18,7 +18,7 @@ module RuboCop module Cop module Graphql - class JSONType < RuboCop::Cop::Cop + class JSONType < RuboCop::Cop::Base MSG = 'Avoid using GraphQL::Types::JSON. See: ' \ 'https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#json' @@ -32,7 +32,7 @@ module RuboCop PATTERN def on_send(node) - add_offense(node, location: :expression) if has_json_type?(node) + add_offense(node) if has_json_type?(node) end end end diff --git a/rubocop/cop/graphql/old_types.rb b/rubocop/cop/graphql/old_types.rb index 61e8ac92dc7..9bbda4732dd 100644 --- a/rubocop/cop/graphql/old_types.rb +++ b/rubocop/cop/graphql/old_types.rb @@ -19,7 +19,7 @@ module RuboCop module Cop module Graphql - class OldTypes < RuboCop::Cop::Cop + class OldTypes < RuboCop::Cop::Base MSG_ID = 'Avoid using GraphQL::ID_TYPE. Use GraphQL::Types::ID instead' MSG_INT = 'Avoid using GraphQL::INT_TYPE. Use GraphQL::Types::Int instead' MSG_STRING = 'Avoid using GraphQL::STRING_TYPE. Use GraphQL::Types::String instead' @@ -37,7 +37,7 @@ module RuboCop old_constant = has_old_type?(node) return unless old_constant - add_offense(node, location: :expression, message: "#{self.class}::MSG_#{old_constant[0..-6]}".constantize) + add_offense(node, message: "#{self.class}::MSG_#{old_constant[0..-6]}".constantize) end end end diff --git a/rubocop/cop/graphql/resolver_type.rb b/rubocop/cop/graphql/resolver_type.rb index e9fa768fd3e..34f1d43fc2a 100644 --- a/rubocop/cop/graphql/resolver_type.rb +++ b/rubocop/cop/graphql/resolver_type.rb @@ -23,7 +23,7 @@ module RuboCop module Cop module Graphql - class ResolverType < RuboCop::Cop::Cop + class ResolverType < RuboCop::Cop::Base MSG = 'Missing type annotation: Please add `type` DSL method call. ' \ 'e.g: type UserType.connection_type, null: true' @@ -32,7 +32,7 @@ module RuboCop PATTERN def on_class(node) - add_offense(node, location: :expression) if resolver?(node) && !typed?(node) + add_offense(node) if resolver?(node) && !typed?(node) end private diff --git a/rubocop/cop/group_public_or_visible_to_user.rb b/rubocop/cop/group_public_or_visible_to_user.rb index beda0b7f8ba..d3aa230680b 100644 --- a/rubocop/cop/group_public_or_visible_to_user.rb +++ b/rubocop/cop/group_public_or_visible_to_user.rb @@ -3,7 +3,7 @@ module RuboCop module Cop # Cop that blacklists the usage of Group.public_or_visible_to_user - class GroupPublicOrVisibleToUser < RuboCop::Cop::Cop + class GroupPublicOrVisibleToUser < RuboCop::Cop::Base MSG = '`Group.public_or_visible_to_user` should be used with extreme care. ' \ 'Please ensure that you are not using it on its own and that the amount ' \ 'of rows being filtered is reasonable.' @@ -15,7 +15,7 @@ module RuboCop def on_send(node) return unless public_or_visible_to_user?(node) - add_offense(node, location: :expression) + add_offense(node) end end end diff --git a/rubocop/cop/ignored_columns.rb b/rubocop/cop/ignored_columns.rb index 4a6f1e4f2d9..542d78c59e9 100644 --- a/rubocop/cop/ignored_columns.rb +++ b/rubocop/cop/ignored_columns.rb @@ -3,7 +3,7 @@ module RuboCop module Cop # Cop that blacklists the usage of `ActiveRecord::Base.ignored_columns=` directly - class IgnoredColumns < RuboCop::Cop::Cop + 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.' @@ -22,11 +22,11 @@ module RuboCop def on_send(node) if ignored_columns?(node) - add_offense(node, location: :expression, message: USE_CONCERN_MSG) + add_offense(node, message: USE_CONCERN_MSG) end if using_ignore?(node) && used_in_wrong_model? - add_offense(node, location: :expression, message: WRONG_MODEL_MSG) + add_offense(node, message: WRONG_MODEL_MSG) end end diff --git a/rubocop/cop/include_sidekiq_worker.rb b/rubocop/cop/include_sidekiq_worker.rb index e39b8bf92c2..1a42de64aa9 100644 --- a/rubocop/cop/include_sidekiq_worker.rb +++ b/rubocop/cop/include_sidekiq_worker.rb @@ -3,7 +3,9 @@ module RuboCop module Cop # Cop that makes sure workers include `ApplicationWorker`, not `Sidekiq::Worker`. - class IncludeSidekiqWorker < RuboCop::Cop::Cop + class IncludeSidekiqWorker < RuboCop::Cop::Base + extend RuboCop::Cop::AutoCorrector + MSG = 'Include `ApplicationWorker`, not `Sidekiq::Worker`.' def_node_matcher :includes_sidekiq_worker?, <<~PATTERN @@ -13,12 +15,8 @@ module RuboCop def on_send(node) return unless includes_sidekiq_worker?(node) - add_offense(node.arguments.first, location: :expression) - end - - def autocorrect(node) - lambda do |corrector| - corrector.replace(node.source_range, 'ApplicationWorker') + add_offense(node.arguments.first) do |corrector| + corrector.replace(node.arguments.first, 'ApplicationWorker') end end end diff --git a/rubocop/cop/inject_enterprise_edition_module.rb b/rubocop/cop/inject_enterprise_edition_module.rb index 785f1494354..7e3c44cc5fd 100644 --- a/rubocop/cop/inject_enterprise_edition_module.rb +++ b/rubocop/cop/inject_enterprise_edition_module.rb @@ -4,7 +4,9 @@ module RuboCop module Cop # Cop that blacklists the injecting of extension specific modules before any lines which are not already injecting another module. # It allows multiple module injections as long as they're all at the end. - class InjectEnterpriseEditionModule < RuboCop::Cop::Cop + class InjectEnterpriseEditionModule < RuboCop::Cop::Base + extend RuboCop::Cop::AutoCorrector + INVALID_LINE = 'Injecting extension modules must be done on the last line of this file' \ ', outside of any class or module definitions' @@ -34,7 +36,7 @@ module RuboCop return unless check_method?(node) if DISALLOW_METHODS.include?(node.children[1]) - add_offense(node, message: DISALLOWED_METHOD) + add_offense(node, message: DISALLOWED_METHOD, &corrector(node)) else verify_line_number(node) verify_argument_type(node) @@ -52,7 +54,7 @@ module RuboCop if allowed_line ignore_node(node) elsif line < last_line - add_offense(node, message: INVALID_LINE) + add_offense(node, message: INVALID_LINE, &corrector(node)) end end @@ -61,7 +63,7 @@ module RuboCop return if argument.str_type? - add_offense(argument, message: INVALID_ARGUMENT) + add_offense(argument, message: INVALID_ARGUMENT, &corrector(argument)) end def check_method?(node) @@ -74,10 +76,12 @@ module RuboCop end end + private + # Automatically correcting these offenses is not always possible, as # sometimes code needs to be refactored to make this work. As such, we # only allow developers to easily blacklist existing offenses. - def autocorrect(node) + def corrector(node) lambda do |corrector| corrector.insert_after( node.source_range, diff --git a/rubocop/cop/lint/last_keyword_argument.rb b/rubocop/cop/lint/last_keyword_argument.rb index 80f4660eeb8..3f5ad7e20d7 100644 --- a/rubocop/cop/lint/last_keyword_argument.rb +++ b/rubocop/cop/lint/last_keyword_argument.rb @@ -9,7 +9,9 @@ module RuboCop # 1. Running specs with RECORD_DEPRECATIONS=1 # 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 + class LastKeywordArgument < RuboCop::Cop::Base + extend RuboCop::Cop::AutoCorrector + MSG = 'Using the last argument as keyword parameters is deprecated' DEPRECATIONS_GLOB = File.expand_path('../../../deprecations/**/*.yml', __dir__) @@ -26,11 +28,7 @@ module RuboCop # parser thinks `a: :b, c: :d` is hash type, it's actually kwargs return if arg.hash_type? && !arg.source.match(/\A{/) - add_offense(arg) - end - - def autocorrect(arg) - lambda do |corrector| + add_offense(arg) do |corrector| if arg.hash_type? kwarg = arg.source.sub(/\A{\s*/, '').sub(/\s*}\z/, '') corrector.replace(arg, kwarg) diff --git a/rubocop/cop/migration/add_column_with_default.rb b/rubocop/cop/migration/add_column_with_default.rb index afd7d93cd47..36603e09263 100644 --- a/rubocop/cop/migration/add_column_with_default.rb +++ b/rubocop/cop/migration/add_column_with_default.rb @@ -5,7 +5,7 @@ require_relative '../../migration_helpers' module RuboCop module Cop module Migration - class AddColumnWithDefault < RuboCop::Cop::Cop + class AddColumnWithDefault < RuboCop::Cop::Base include MigrationHelpers MSG = '`add_column_with_default` is deprecated, use `add_column` instead' @@ -15,7 +15,7 @@ module RuboCop name = node.children[1] - add_offense(node, location: :selector) if name == :add_column_with_default + add_offense(node.loc.selector) if name == :add_column_with_default end end end diff --git a/rubocop/cop/migration/add_columns_to_wide_tables.rb b/rubocop/cop/migration/add_columns_to_wide_tables.rb index 41056379515..98dd605faef 100644 --- a/rubocop/cop/migration/add_columns_to_wide_tables.rb +++ b/rubocop/cop/migration/add_columns_to_wide_tables.rb @@ -6,7 +6,7 @@ module RuboCop module Cop module Migration # Cop that prevents adding columns to wide tables. - class AddColumnsToWideTables < RuboCop::Cop::Cop + class AddColumnsToWideTables < RuboCop::Cop::Base include MigrationHelpers MSG = '`%s` is a wide table with several columns, adding more should be avoided unless absolutely necessary.' \ @@ -26,7 +26,7 @@ module RuboCop return unless offense?(method_name, table_name) - add_offense(node, location: :selector, message: format(MSG, table_name.value)) + add_offense(node.loc.selector, message: format(MSG, table_name.value)) end private diff --git a/rubocop/cop/migration/add_concurrent_foreign_key.rb b/rubocop/cop/migration/add_concurrent_foreign_key.rb index ebab6aa653e..f538207d336 100644 --- a/rubocop/cop/migration/add_concurrent_foreign_key.rb +++ b/rubocop/cop/migration/add_concurrent_foreign_key.rb @@ -7,7 +7,7 @@ module RuboCop module Migration # Cop that checks if `add_concurrent_foreign_key` is used instead of # `add_foreign_key`. - class AddConcurrentForeignKey < RuboCop::Cop::Cop + class AddConcurrentForeignKey < RuboCop::Cop::Base include MigrationHelpers MSG = '`add_foreign_key` requires downtime, use `add_concurrent_foreign_key` instead' @@ -29,7 +29,7 @@ module RuboCop return if in_with_lock_retries?(node) return if not_valid_fk?(node) - add_offense(node, location: :selector) + add_offense(node.loc.selector) end def method_name(node) diff --git a/rubocop/cop/migration/add_concurrent_index.rb b/rubocop/cop/migration/add_concurrent_index.rb index bfe7c15bfdf..77b8ced4778 100644 --- a/rubocop/cop/migration/add_concurrent_index.rb +++ b/rubocop/cop/migration/add_concurrent_index.rb @@ -7,7 +7,7 @@ module RuboCop module Migration # Cop that checks if `add_concurrent_index` is used with `up`/`down` methods # and not `change`. - class AddConcurrentIndex < RuboCop::Cop::Cop + class AddConcurrentIndex < RuboCop::Cop::Base include MigrationHelpers MSG = '`add_concurrent_index` is not reversible so you must manually define ' \ @@ -21,15 +21,11 @@ module RuboCop return unless name == :add_concurrent_index node.each_ancestor(:def) do |def_node| - next unless method_name(def_node) == :change + next unless def_node.method_name == :change - add_offense(def_node, location: :name) + add_offense(def_node.loc.name) end end - - def method_name(node) - node.children.first - end end end end diff --git a/rubocop/cop/migration/add_index.rb b/rubocop/cop/migration/add_index.rb index 327e89fb040..3920fd19c98 100644 --- a/rubocop/cop/migration/add_index.rb +++ b/rubocop/cop/migration/add_index.rb @@ -6,7 +6,7 @@ module RuboCop module Cop module Migration # Cop that checks if indexes are added in a concurrent manner. - class AddIndex < RuboCop::Cop::Cop + class AddIndex < RuboCop::Cop::Base include MigrationHelpers MSG = '`add_index` requires downtime, use `add_concurrent_index` instead' @@ -29,7 +29,7 @@ module RuboCop # data in these tables yet. next if new_tables.include?(first_arg) - add_offense(send_node, location: :selector) + add_offense(send_node.loc.selector) end end diff --git a/rubocop/cop/migration/add_limit_to_text_columns.rb b/rubocop/cop/migration/add_limit_to_text_columns.rb index a47fbe0bf16..5c71fbbfaa2 100644 --- a/rubocop/cop/migration/add_limit_to_text_columns.rb +++ b/rubocop/cop/migration/add_limit_to_text_columns.rb @@ -10,7 +10,7 @@ module RuboCop # Text columns starting with `encrypted_` are very likely used # by `attr_encrypted` which controls the text length. Those columns # should not add a text limit. - class AddLimitToTextColumns < RuboCop::Cop::Cop + class AddLimitToTextColumns < RuboCop::Cop::Base include MigrationHelpers TEXT_LIMIT_ATTRIBUTE_ALLOWED_SINCE = 2021_09_10_00_00_00 @@ -43,11 +43,11 @@ module RuboCop next unless text_operation?(send_node) if text_operation_with_limit?(send_node) - add_offense(send_node, location: :selector, message: TEXT_LIMIT_ATTRIBUTE_NOT_ALLOWED) if version(node) < TEXT_LIMIT_ATTRIBUTE_ALLOWED_SINCE + add_offense(send_node.loc.selector, message: TEXT_LIMIT_ATTRIBUTE_NOT_ALLOWED) if version(node) < TEXT_LIMIT_ATTRIBUTE_ALLOWED_SINCE else # We require a limit for the same table and attribute name if text_limit_missing?(node, *table_and_attribute_name(send_node)) - add_offense(send_node, location: :selector) + add_offense(send_node.loc.selector) end end end diff --git a/rubocop/cop/migration/add_reference.rb b/rubocop/cop/migration/add_reference.rb index 33840fc7caf..02a0ef899b4 100644 --- a/rubocop/cop/migration/add_reference.rb +++ b/rubocop/cop/migration/add_reference.rb @@ -6,7 +6,7 @@ module RuboCop module Migration # add_reference can only be used with newly created tables. # Additionally, the cop here checks that we create an index for the foreign key, too. - class AddReference < RuboCop::Cop::Cop + class AddReference < RuboCop::Cop::Base include MigrationHelpers MSG = '`add_reference` requires downtime for existing tables, use `add_concurrent_foreign_key` instead. When used for new tables, `index: true` or `index: { options... } is required.`' @@ -28,12 +28,12 @@ module RuboCop # Using "add_reference" is fine for newly created tables as there's no # data in these tables yet. if existing_table?(new_tables, first_arg) - add_offense(send_node, location: :selector) + add_offense(send_node.loc.selector) end # We require an index on the foreign key column. if index_missing?(node) - add_offense(send_node, location: :selector) + add_offense(send_node.loc.selector) end end end diff --git a/rubocop/cop/migration/add_timestamps.rb b/rubocop/cop/migration/add_timestamps.rb index 01d3f01ef4f..8b09d730cbc 100644 --- a/rubocop/cop/migration/add_timestamps.rb +++ b/rubocop/cop/migration/add_timestamps.rb @@ -6,7 +6,7 @@ module RuboCop module Cop module Migration # Cop that checks if 'add_timestamps' method is called with timezone information. - class AddTimestamps < RuboCop::Cop::Cop + class AddTimestamps < RuboCop::Cop::Base include MigrationHelpers MSG = 'Do not use `add_timestamps`, use `add_timestamps_with_timezone` instead' @@ -15,7 +15,7 @@ module RuboCop def on_send(node) return unless in_migration?(node) - add_offense(node, location: :selector) if method_name(node) == :add_timestamps + add_offense(node.loc.selector) if method_name(node) == :add_timestamps end def method_name(node) diff --git a/rubocop/cop/migration/background_migration_base_class.rb b/rubocop/cop/migration/background_migration_base_class.rb index 50cbe3a69c3..9ec8403a607 100644 --- a/rubocop/cop/migration/background_migration_base_class.rb +++ b/rubocop/cop/migration/background_migration_base_class.rb @@ -3,7 +3,7 @@ module RuboCop module Cop module Migration - class BackgroundMigrationBaseClass < RuboCop::Cop::Cop + class BackgroundMigrationBaseClass < RuboCop::Cop::Base MSG = 'Batched background migration jobs should inherit from Gitlab::BackgroundMigration::BatchedMigrationJob' def_node_search :top_level_module?, <<~PATTERN @@ -25,7 +25,7 @@ module RuboCop return if top_level_class_node.nil? || inherits_batched_migration_job?(top_level_class_node) - add_offense(top_level_class_node, location: :expression) + add_offense(top_level_class_node) end end end diff --git a/rubocop/cop/migration/background_migration_record.rb b/rubocop/cop/migration/background_migration_record.rb index 2ee6b9f7b42..ee6f3fd140b 100644 --- a/rubocop/cop/migration/background_migration_record.rb +++ b/rubocop/cop/migration/background_migration_record.rb @@ -5,7 +5,7 @@ require_relative '../../migration_helpers' module RuboCop module Cop module Migration - class BackgroundMigrationRecord < RuboCop::Cop::Cop + class BackgroundMigrationRecord < RuboCop::Cop::Base include MigrationHelpers MSG = <<~MSG @@ -26,14 +26,14 @@ module RuboCop return unless in_background_migration?(node) return unless inherits_from_active_record_base?(node) - add_offense(node, location: :expression) + add_offense(node) end def on_send(node) return unless in_background_migration?(node) return unless class_new_active_record_base?(node) - add_offense(node, location: :expression) + add_offense(node) end end end diff --git a/rubocop/cop/migration/background_migrations.rb b/rubocop/cop/migration/background_migrations.rb index 39c5496e4d3..7dcc2101fb1 100644 --- a/rubocop/cop/migration/background_migrations.rb +++ b/rubocop/cop/migration/background_migrations.rb @@ -5,7 +5,7 @@ require_relative '../../migration_helpers' module RuboCop module Cop module Migration - class BackgroundMigrations < RuboCop::Cop::Cop + class BackgroundMigrations < RuboCop::Cop::Base include MigrationHelpers MSG = 'Background migrations are deprecated. Please use a Batched Background Migration instead.'\ @@ -20,7 +20,7 @@ module RuboCop migrate_in ) - add_offense(node, location: :selector) if disabled_methods.include? name + add_offense(node.loc.selector) if disabled_methods.include? name end end end diff --git a/rubocop/cop/migration/complex_indexes_require_name.rb b/rubocop/cop/migration/complex_indexes_require_name.rb index 82deb36716d..771537f2576 100644 --- a/rubocop/cop/migration/complex_indexes_require_name.rb +++ b/rubocop/cop/migration/complex_indexes_require_name.rb @@ -5,7 +5,7 @@ require_relative '../../migration_helpers' module RuboCop module Cop module Migration - class ComplexIndexesRequireName < RuboCop::Cop::Cop + class ComplexIndexesRequireName < RuboCop::Cop::Base include MigrationHelpers MSG = 'indexes added with custom options must be explicitly named' @@ -32,7 +32,7 @@ module RuboCop node.each_descendant(:send) do |send_node| next unless create_table_with_index_offense?(send_node) || add_index_offense?(send_node) - add_offense(send_node, location: :selector) + add_offense(send_node.loc.selector) end end diff --git a/rubocop/cop/migration/create_table_with_foreign_keys.rb b/rubocop/cop/migration/create_table_with_foreign_keys.rb index 382a2d6f65b..e3039ac76a9 100644 --- a/rubocop/cop/migration/create_table_with_foreign_keys.rb +++ b/rubocop/cop/migration/create_table_with_foreign_keys.rb @@ -5,7 +5,7 @@ require_relative '../../migration_helpers' module RuboCop module Cop module Migration - class CreateTableWithForeignKeys < RuboCop::Cop::Cop + class CreateTableWithForeignKeys < RuboCop::Cop::Base include MigrationHelpers MSG = 'Creating a table with more than one foreign key at once violates our migration style guide. ' \ diff --git a/rubocop/cop/migration/datetime.rb b/rubocop/cop/migration/datetime.rb index c605c8e1b6e..9cdb2d83ab5 100644 --- a/rubocop/cop/migration/datetime.rb +++ b/rubocop/cop/migration/datetime.rb @@ -6,7 +6,7 @@ module RuboCop module Cop module Migration # Cop that checks if datetime data type is added with timezone information. - class Datetime < RuboCop::Cop::Cop + class Datetime < RuboCop::Cop::Base include MigrationHelpers MSG = 'Do not use the `%s` data type, use `datetime_with_timezone` instead' @@ -19,7 +19,7 @@ module RuboCop method_name = send_node.children[1] if method_name == :datetime || method_name == :timestamp - add_offense(send_node, location: :selector, message: format(MSG, method_name)) + add_offense(send_node.loc.selector, message: format(MSG, method_name)) end end end @@ -34,7 +34,7 @@ module RuboCop last_argument = descendant.children.last if last_argument == :datetime || last_argument == :timestamp - add_offense(node, location: :expression, message: format(MSG, last_argument)) + add_offense(node, message: format(MSG, last_argument)) end end end diff --git a/rubocop/cop/migration/drop_table.rb b/rubocop/cop/migration/drop_table.rb index 531cbb14021..62dea79cbe6 100644 --- a/rubocop/cop/migration/drop_table.rb +++ b/rubocop/cop/migration/drop_table.rb @@ -7,7 +7,7 @@ module RuboCop module Migration # Cop that checks if `drop_table` is called in deployment migrations. # Calling it in deployment migrations can cause downtimes as there still may be code using the target tables. - class DropTable < RuboCop::Cop::Cop + class DropTable < RuboCop::Cop::Base include MigrationHelpers MSG = <<-MESSAGE.delete("\n").squeeze @@ -22,7 +22,7 @@ module RuboCop node.each_descendant(:send) do |send_node| next unless offensible?(send_node) - add_offense(send_node, location: :selector) + add_offense(send_node.loc.selector) end end diff --git a/rubocop/cop/migration/migration_record.rb b/rubocop/cop/migration/migration_record.rb index 291644f10e3..1543d2fff08 100644 --- a/rubocop/cop/migration/migration_record.rb +++ b/rubocop/cop/migration/migration_record.rb @@ -5,7 +5,7 @@ require_relative '../../migration_helpers' module RuboCop module Cop module Migration - class MigrationRecord < RuboCop::Cop::Cop + class MigrationRecord < RuboCop::Cop::Base include MigrationHelpers ENFORCED_SINCE = 2022_04_26_00_00_00 @@ -27,7 +27,7 @@ module RuboCop return unless relevant_migration?(node) return unless inherits_from_active_record_base?(node) || inherits_from_application_record?(node) - add_offense(node, location: :expression) + add_offense(node) end private diff --git a/rubocop/cop/migration/prevent_global_enable_lock_retries_with_disable_ddl_transaction.rb b/rubocop/cop/migration/prevent_global_enable_lock_retries_with_disable_ddl_transaction.rb index f5343f973e1..9556a16dc01 100644 --- a/rubocop/cop/migration/prevent_global_enable_lock_retries_with_disable_ddl_transaction.rb +++ b/rubocop/cop/migration/prevent_global_enable_lock_retries_with_disable_ddl_transaction.rb @@ -6,7 +6,7 @@ module RuboCop module Cop module Migration # Cop that prevents usage of `enable_lock_retries!` within the `disable_ddl_transaction!` method. - class PreventGlobalEnableLockRetriesWithDisableDdlTransaction < RuboCop::Cop::Cop + class PreventGlobalEnableLockRetriesWithDisableDdlTransaction < RuboCop::Cop::Base include MigrationHelpers MSG = '`enable_lock_retries!` cannot be used with `disable_ddl_transaction!`. Use the `with_lock_retries` helper method to define retriable code blocks.' diff --git a/rubocop/cop/migration/prevent_index_creation.rb b/rubocop/cop/migration/prevent_index_creation.rb index c383466f73b..185f36cb24b 100644 --- a/rubocop/cop/migration/prevent_index_creation.rb +++ b/rubocop/cop/migration/prevent_index_creation.rb @@ -5,7 +5,7 @@ module RuboCop module Cop module Migration # Cop that checks if new indexes are introduced to forbidden tables. - class PreventIndexCreation < RuboCop::Cop::Cop + class PreventIndexCreation < RuboCop::Cop::Base include MigrationHelpers FORBIDDEN_TABLES = %i[ci_builds].freeze @@ -41,7 +41,7 @@ module RuboCop return unless in_migration?(node) node.each_descendant(:send) do |send_node| - add_offense(send_node, location: :selector) if offense?(send_node) + add_offense(send_node.loc.selector) if offense?(send_node) end end diff --git a/rubocop/cop/migration/prevent_strings.rb b/rubocop/cop/migration/prevent_strings.rb index 57e29bf74ae..89659050f64 100644 --- a/rubocop/cop/migration/prevent_strings.rb +++ b/rubocop/cop/migration/prevent_strings.rb @@ -6,7 +6,7 @@ module RuboCop module Cop module Migration # Cop that enforces using text instead of the string data type - class PreventStrings < RuboCop::Cop::Cop + class PreventStrings < RuboCop::Cop::Base include MigrationHelpers MSG = 'Do not use the `string` data type, use `text` instead. ' \ @@ -26,7 +26,7 @@ module RuboCop node.each_descendant(:send) do |send_node| next unless string_operation?(send_node) - add_offense(send_node, location: :selector) + add_offense(send_node.loc.selector) end end diff --git a/rubocop/cop/migration/refer_to_index_by_name.rb b/rubocop/cop/migration/refer_to_index_by_name.rb index fbf3c0d7030..78619472487 100644 --- a/rubocop/cop/migration/refer_to_index_by_name.rb +++ b/rubocop/cop/migration/refer_to_index_by_name.rb @@ -5,7 +5,7 @@ require_relative '../../migration_helpers' module RuboCop module Cop module Migration - class ReferToIndexByName < RuboCop::Cop::Cop + class ReferToIndexByName < RuboCop::Cop::Base include MigrationHelpers MSG = 'migration methods that refer to existing indexes must do so by name' @@ -32,7 +32,7 @@ module RuboCop node.each_descendant(:send) do |send_node| next unless index_exists_offense?(send_node) || removing_index_offense?(send_node) - add_offense(send_node, location: :selector) + add_offense(send_node.loc.selector) end end diff --git a/rubocop/cop/migration/remove_column.rb b/rubocop/cop/migration/remove_column.rb index 6a171ac948f..e8477a211e4 100644 --- a/rubocop/cop/migration/remove_column.rb +++ b/rubocop/cop/migration/remove_column.rb @@ -7,7 +7,7 @@ module RuboCop module Migration # Cop that checks if remove_column is used in a regular (not # post-deployment) migration. - class RemoveColumn < RuboCop::Cop::Cop + class RemoveColumn < RuboCop::Cop::Base include MigrationHelpers MSG = '`remove_column` must only be used in post-deployment migrations' @@ -22,7 +22,7 @@ module RuboCop send_method = send_node.children[1] if send_method == :remove_column - add_offense(send_node, location: :selector) + add_offense(send_node.loc.selector) end end end diff --git a/rubocop/cop/migration/remove_concurrent_index.rb b/rubocop/cop/migration/remove_concurrent_index.rb index 30dd59d97bc..459f474d185 100644 --- a/rubocop/cop/migration/remove_concurrent_index.rb +++ b/rubocop/cop/migration/remove_concurrent_index.rb @@ -7,7 +7,7 @@ module RuboCop module Migration # Cop that checks if `remove_concurrent_index` is used with `up`/`down` methods # and not `change`. - class RemoveConcurrentIndex < RuboCop::Cop::Cop + class RemoveConcurrentIndex < RuboCop::Cop::Base include MigrationHelpers MSG = '`remove_concurrent_index` is not reversible so you must manually define ' \ @@ -18,13 +18,9 @@ module RuboCop return unless node.children[1] == :remove_concurrent_index node.each_ancestor(:def) do |def_node| - add_offense(def_node, location: :name) if method_name(def_node) == :change + add_offense(def_node.loc.name) if def_node.method_name == :change end end - - def method_name(node) - node.children[0] - end end end end diff --git a/rubocop/cop/migration/remove_index.rb b/rubocop/cop/migration/remove_index.rb index ca5d4af1520..26d271207dc 100644 --- a/rubocop/cop/migration/remove_index.rb +++ b/rubocop/cop/migration/remove_index.rb @@ -6,7 +6,7 @@ module RuboCop module Cop module Migration # Cop that checks if indexes are removed in a concurrent manner. - class RemoveIndex < RuboCop::Cop::Cop + class RemoveIndex < RuboCop::Cop::Base include MigrationHelpers MSG = '`remove_index` requires downtime, use `remove_concurrent_index` instead' @@ -15,7 +15,7 @@ module RuboCop return unless in_migration?(node) node.each_descendant(:send) do |send_node| - add_offense(send_node, location: :selector) if method_name(send_node) == :remove_index + add_offense(send_node.loc.selector) if method_name(send_node) == :remove_index end end diff --git a/rubocop/cop/migration/safer_boolean_column.rb b/rubocop/cop/migration/safer_boolean_column.rb index 1d780d96afa..d3d77b16357 100644 --- a/rubocop/cop/migration/safer_boolean_column.rb +++ b/rubocop/cop/migration/safer_boolean_column.rb @@ -18,7 +18,7 @@ module RuboCop # # See https://gitlab.com/gitlab-org/gitlab/issues/2750 for more # information. - class SaferBooleanColumn < RuboCop::Cop::Cop + class SaferBooleanColumn < RuboCop::Cop::Base include MigrationHelpers DEFAULT_OFFENSE = 'Boolean columns on the `%s` table should have a default. You may wish to use `add_column_with_default`.' @@ -52,7 +52,7 @@ module RuboCop NULL_OFFENSE end - add_offense(node, location: :expression, message: format(offense, table)) if offense + add_offense(node, message: format(offense, table)) if offense end def no_default?(opts) diff --git a/rubocop/cop/migration/schedule_async.rb b/rubocop/cop/migration/schedule_async.rb index 4fdedecdf64..bc98d45b9c6 100644 --- a/rubocop/cop/migration/schedule_async.rb +++ b/rubocop/cop/migration/schedule_async.rb @@ -5,7 +5,7 @@ require_relative '../../migration_helpers' module RuboCop module Cop module Migration - class ScheduleAsync < RuboCop::Cop::Cop + class ScheduleAsync < RuboCop::Cop::Base include MigrationHelpers ENFORCED_SINCE = 2020_02_12_00_00_00 @@ -32,7 +32,7 @@ module RuboCop return if version(node) < ENFORCED_SINCE return unless calls_background_migration_worker?(node) || calls_ci_database_worker?(node) - add_offense(node, location: :expression) + add_offense(node) end end end diff --git a/rubocop/cop/migration/sidekiq_queue_migrate.rb b/rubocop/cop/migration/sidekiq_queue_migrate.rb index 134bda590da..4a7aaa353a3 100644 --- a/rubocop/cop/migration/sidekiq_queue_migrate.rb +++ b/rubocop/cop/migration/sidekiq_queue_migrate.rb @@ -7,7 +7,7 @@ module RuboCop module Migration # Cop that checks if sidekiq_queue_migrate is used in a regular # (not post-deployment) migration. - class SidekiqQueueMigrate < RuboCop::Cop::Cop + class SidekiqQueueMigrate < RuboCop::Cop::Base include MigrationHelpers MSG = '`sidekiq_queue_migrate` must only be used in post-deployment migrations' @@ -19,7 +19,7 @@ module RuboCop send_method = send_node.children[1] if send_method == :sidekiq_queue_migrate - add_offense(send_node, location: :selector) + add_offense(send_node.loc.selector) end end end diff --git a/rubocop/cop/migration/timestamps.rb b/rubocop/cop/migration/timestamps.rb index 44baf17d968..5cc78a8821a 100644 --- a/rubocop/cop/migration/timestamps.rb +++ b/rubocop/cop/migration/timestamps.rb @@ -6,7 +6,7 @@ module RuboCop module Cop module Migration # Cop that checks if 'timestamps' method is called with timezone information. - class Timestamps < RuboCop::Cop::Cop + class Timestamps < RuboCop::Cop::Base include MigrationHelpers MSG = 'Do not use `timestamps`, use `timestamps_with_timezone` instead' @@ -16,7 +16,7 @@ module RuboCop return unless in_migration?(node) node.each_descendant(:send) do |send_node| - add_offense(send_node, location: :selector) if method_name(send_node) == :timestamps + add_offense(send_node.loc.selector) if method_name(send_node) == :timestamps end end diff --git a/rubocop/cop/migration/update_column_in_batches.rb b/rubocop/cop/migration/update_column_in_batches.rb index e23042e1b9f..7f4479c62e3 100644 --- a/rubocop/cop/migration/update_column_in_batches.rb +++ b/rubocop/cop/migration/update_column_in_batches.rb @@ -7,7 +7,7 @@ module RuboCop module Migration # Cop that checks if a spec file exists for any migration using # `update_column_in_batches`. - class UpdateColumnInBatches < RuboCop::Cop::Cop + class UpdateColumnInBatches < RuboCop::Cop::Base include MigrationHelpers MSG = 'Migration running `update_column_in_batches` must have a spec file at' \ @@ -19,9 +19,9 @@ module RuboCop spec_path = spec_filename(node) - unless File.exist?(File.expand_path(spec_path, rails_root)) - add_offense(node, location: :expression, message: format(MSG, spec_path)) - end + return if File.exist?(File.expand_path(spec_path, rails_root)) + + add_offense(node, message: format(MSG, spec_path)) end private diff --git a/rubocop/cop/migration/versioned_migration_class.rb b/rubocop/cop/migration/versioned_migration_class.rb index f2e4550c691..648782f1735 100644 --- a/rubocop/cop/migration/versioned_migration_class.rb +++ b/rubocop/cop/migration/versioned_migration_class.rb @@ -5,7 +5,7 @@ require_relative '../../migration_helpers' module RuboCop module Cop module Migration - class VersionedMigrationClass < RuboCop::Cop::Cop + class VersionedMigrationClass < RuboCop::Cop::Base include MigrationHelpers ENFORCED_SINCE = 2021_09_02_00_00_00 @@ -26,13 +26,13 @@ module RuboCop return unless relevant_migration?(node) return unless activerecord_migration_class?(node) - add_offense(node, location: :expression, message: MSG_INHERIT) + add_offense(node, message: MSG_INHERIT) end def on_send(node) return unless relevant_migration?(node) - add_offense(node, location: :expression, message: MSG_INCLUDE) if includes_helpers?(node) + add_offense(node, message: MSG_INCLUDE) if includes_helpers?(node) end private diff --git a/rubocop/cop/migration/with_lock_retries_disallowed_method.rb b/rubocop/cop/migration/with_lock_retries_disallowed_method.rb index b3d05ad1a6d..5c96b38dcdc 100644 --- a/rubocop/cop/migration/with_lock_retries_disallowed_method.rb +++ b/rubocop/cop/migration/with_lock_retries_disallowed_method.rb @@ -5,7 +5,7 @@ require_relative '../../migration_helpers' module RuboCop module Cop module Migration - class WithLockRetriesDisallowedMethod < RuboCop::Cop::Cop + class WithLockRetriesDisallowedMethod < RuboCop::Cop::Base include MigrationHelpers ALLOWED_MIGRATION_METHODS = %i[ @@ -60,8 +60,8 @@ module RuboCop return unless send_node?(node) name = node.children[1] - add_offense(node, location: :expression) unless ALLOWED_MIGRATION_METHODS.include?(name) - add_offense(node, location: :selector, message: MSG_ONLY_ONE_FK_ALLOWED) if multiple_fks?(node) + add_offense(node) unless ALLOWED_MIGRATION_METHODS.include?(name) + add_offense(node.loc.selector, message: MSG_ONLY_ONE_FK_ALLOWED) if multiple_fks?(node) end def multiple_fks?(node) diff --git a/rubocop/cop/migration/with_lock_retries_with_change.rb b/rubocop/cop/migration/with_lock_retries_with_change.rb index 9d11edcb6a1..59dbd6f22d2 100644 --- a/rubocop/cop/migration/with_lock_retries_with_change.rb +++ b/rubocop/cop/migration/with_lock_retries_with_change.rb @@ -6,7 +6,7 @@ module RuboCop module Cop module Migration # Cop that prevents usage of `with_lock_retries` within the `change` method. - class WithLockRetriesWithChange < RuboCop::Cop::Cop + class WithLockRetriesWithChange < RuboCop::Cop::Base include MigrationHelpers MSG = '`with_lock_retries` cannot be used within `change` so you must manually define ' \ @@ -17,13 +17,9 @@ module RuboCop return unless node.children[1] == :with_lock_retries node.each_ancestor(:def) do |def_node| - add_offense(def_node, location: :name) if method_name(def_node) == :change + add_offense(def_node.loc.name) if def_node.method_name == :change end end - - def method_name(node) - node.children.first - end end end end diff --git a/rubocop/cop/performance/active_record_subtransaction_methods.rb b/rubocop/cop/performance/active_record_subtransaction_methods.rb index 3b89d3ab858..1789e20ce45 100644 --- a/rubocop/cop/performance/active_record_subtransaction_methods.rb +++ b/rubocop/cop/performance/active_record_subtransaction_methods.rb @@ -5,7 +5,7 @@ module RuboCop module Performance # Cop that disallows certain methods that rely on subtransactions in their implementation. # Companion to Performance/ActiveRecordSubtransactions, which bans direct usage of subtransactions. - class ActiveRecordSubtransactionMethods < RuboCop::Cop::Cop + class ActiveRecordSubtransactionMethods < RuboCop::Cop::Base MSG = 'Methods that rely on subtransactions should not be used. ' \ 'For more information see: https://gitlab.com/gitlab-org/gitlab/-/issues/338346' @@ -21,7 +21,7 @@ module RuboCop def on_send(node) return unless DISALLOWED_METHODS.include?(node.method_name) - add_offense(node, location: :selector) + add_offense(node.loc.selector) end end end diff --git a/rubocop/cop/performance/active_record_subtransactions.rb b/rubocop/cop/performance/active_record_subtransactions.rb index a550b558e52..165efee7c6b 100644 --- a/rubocop/cop/performance/active_record_subtransactions.rb +++ b/rubocop/cop/performance/active_record_subtransactions.rb @@ -3,7 +3,7 @@ module RuboCop module Cop module Performance - class ActiveRecordSubtransactions < RuboCop::Cop::Cop + class ActiveRecordSubtransactions < RuboCop::Cop::Base MSG = 'Subtransactions should not be used. ' \ 'For more information see: https://gitlab.com/gitlab-org/gitlab/-/issues/338346' diff --git a/rubocop/cop/performance/ar_count_each.rb b/rubocop/cop/performance/ar_count_each.rb index 2fe8e549872..1dae473d87d 100644 --- a/rubocop/cop/performance/ar_count_each.rb +++ b/rubocop/cop/performance/ar_count_each.rb @@ -3,7 +3,7 @@ module RuboCop module Cop module Performance - class ARCountEach < RuboCop::Cop::Cop + class ARCountEach < RuboCop::Cop::Base def message(ivar) "If #{ivar} is AR relation, avoid `#{ivar}.count ...; #{ivar}.each... `, this will trigger two queries. " \ "Use `#{ivar}.load.size ...; #{ivar}.each... ` instead. If #{ivar} is an array, try to use #{ivar}.size." @@ -35,7 +35,7 @@ module RuboCop begin_node.each_descendant do |n| ivar_each = each_match(n) - add_offense(node, location: :expression, message: message(ivar_count)) if ivar_each == ivar_count + add_offense(node, message: message(ivar_count)) if ivar_each == ivar_count end end end diff --git a/rubocop/cop/performance/ar_exists_and_present_blank.rb b/rubocop/cop/performance/ar_exists_and_present_blank.rb index 54c2d6bf95a..c2ba6c2a206 100644 --- a/rubocop/cop/performance/ar_exists_and_present_blank.rb +++ b/rubocop/cop/performance/ar_exists_and_present_blank.rb @@ -3,7 +3,7 @@ module RuboCop module Cop module Performance - class ARExistsAndPresentBlank < RuboCop::Cop::Cop + class ARExistsAndPresentBlank < RuboCop::Cop::Base def message_present(ivar) "Avoid `#{ivar}.present?`, because it will generate database query 'Select TABLE.*' which is expensive. "\ "Suggest to use `#{ivar}.any?` to replace `#{ivar}.present?`" @@ -46,8 +46,8 @@ module RuboCop ivar_exists = exists_match(n) next unless ivar_exists - add_offense(node, location: :expression, message: message_present(ivar_exists)) if ivar_exists == ivar_present - add_offense(node, location: :expression, message: message_blank(ivar_exists)) if ivar_exists == ivar_blank + add_offense(node, message: message_present(ivar_exists)) if ivar_exists == ivar_present + add_offense(node, message: message_blank(ivar_exists)) if ivar_exists == ivar_blank end end end diff --git a/rubocop/cop/performance/readlines_each.rb b/rubocop/cop/performance/readlines_each.rb index cb4ffaca6e9..7a3a15020db 100644 --- a/rubocop/cop/performance/readlines_each.rb +++ b/rubocop/cop/performance/readlines_each.rb @@ -3,7 +3,9 @@ module RuboCop module Cop module Performance - class ReadlinesEach < RuboCop::Cop::Cop + class ReadlinesEach < RuboCop::Cop::Base + extend RuboCop::Cop::AutoCorrector + MESSAGE = 'Avoid `IO.readlines.each`, since it reads contents into memory in full. ' \ 'Use `IO.each_line` or `IO.each` instead.' @@ -17,12 +19,9 @@ module RuboCop PATTERN def on_send(node) - full_file_read_via_class?(node) { add_offense(node, location: :selector, message: MESSAGE) } - full_file_read_via_instance?(node) { add_offense(node, location: :selector, message: MESSAGE) } - end + return unless full_file_read_via_class?(node) || full_file_read_via_instance?(node) - def autocorrect(node) - lambda do |corrector| + add_offense(node.loc.selector, message: MESSAGE) do |corrector| corrector.replace(node.loc.expression, node.source.gsub('readlines.each', 'each_line')) end end diff --git a/rubocop/cop/prefer_class_methods_over_module.rb b/rubocop/cop/prefer_class_methods_over_module.rb index 39b65073477..d865cc6c003 100644 --- a/rubocop/cop/prefer_class_methods_over_module.rb +++ b/rubocop/cop/prefer_class_methods_over_module.rb @@ -26,7 +26,8 @@ module RuboCop # end # end # - class PreferClassMethodsOverModule < RuboCop::Cop::Cop + class PreferClassMethodsOverModule < RuboCop::Cop::Base + extend RuboCop::Cop::AutoCorrector include RangeHelp MSG = 'Do not use module ClassMethods, use class_methods block instead.' @@ -36,17 +37,20 @@ module RuboCop PATTERN def on_module(node) - add_offense(node) if node.defined_module_name == 'ClassMethods' && module_extends_activesupport_concern?(node) - end + return unless class_methods_module_in_activesupport_concern?(node) - def autocorrect(node) - lambda do |corrector| + add_offense(node) do |corrector| corrector.replace(module_range(node), 'class_methods do') end end private + def class_methods_module_in_activesupport_concern?(node) + node.defined_module_name == 'ClassMethods' && + module_extends_activesupport_concern?(node) + end + def module_extends_activesupport_concern?(node) container_module = container_module_of(node) return false unless container_module diff --git a/rubocop/cop/project_path_helper.rb b/rubocop/cop/project_path_helper.rb index 0d12f2d2b12..104a352949f 100644 --- a/rubocop/cop/project_path_helper.rb +++ b/rubocop/cop/project_path_helper.rb @@ -2,7 +2,9 @@ module RuboCop module Cop - class ProjectPathHelper < RuboCop::Cop::Cop + class ProjectPathHelper < RuboCop::Cop::Base + extend RuboCop::Cop::AutoCorrector + 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)`.' @@ -19,18 +21,14 @@ module RuboCop return unless method_name(namespace_expr) == :namespace return unless receiver(namespace_expr) == project_expr - add_offense(node, location: :selector) - end - - def autocorrect(node) - helper_name = method_name(node).to_s.sub('namespace_project', 'project') + add_offense(node.loc.selector) do |corrector| + helper_name = method_name(node).to_s.sub('namespace_project', 'project') - arguments = arguments(node) - arguments.shift # Remove namespace argument + arguments = arguments(node) + arguments.shift # Remove namespace argument - replacement = "#{helper_name}(#{arguments.map(&:source).join(', ')})" + replacement = "#{helper_name}(#{arguments.map(&:source).join(', ')})" - lambda do |corrector| corrector.replace(node.source_range, replacement) end end diff --git a/rubocop/cop/put_group_routes_under_scope.rb b/rubocop/cop/put_group_routes_under_scope.rb index 9adec044da8..83afd112b99 100644 --- a/rubocop/cop/put_group_routes_under_scope.rb +++ b/rubocop/cop/put_group_routes_under_scope.rb @@ -6,7 +6,7 @@ module RuboCop module Cop # Checks for a group routes outside '/-/' scope. # For more information see: https://gitlab.com/gitlab-org/gitlab/issues/29572 - class PutGroupRoutesUnderScope < RuboCop::Cop::Cop + class PutGroupRoutesUnderScope < RuboCop::Cop::Base include RoutesUnderScope MSG = 'Put new group routes under /-/ scope' diff --git a/rubocop/cop/put_project_routes_under_scope.rb b/rubocop/cop/put_project_routes_under_scope.rb index cddb147324f..d3b86bad947 100644 --- a/rubocop/cop/put_project_routes_under_scope.rb +++ b/rubocop/cop/put_project_routes_under_scope.rb @@ -6,7 +6,7 @@ module RuboCop module Cop # Checks for a project routes outside '/-/' scope. # For more information see: https://gitlab.com/gitlab-org/gitlab/issues/29572 - class PutProjectRoutesUnderScope < RuboCop::Cop::Cop + class PutProjectRoutesUnderScope < RuboCop::Cop::Base include RoutesUnderScope MSG = 'Put new project routes under /-/ scope' diff --git a/rubocop/cop/qa/ambiguous_page_object_name.rb b/rubocop/cop/qa/ambiguous_page_object_name.rb index a4a2c04f61f..a331851bcc5 100644 --- a/rubocop/cop/qa/ambiguous_page_object_name.rb +++ b/rubocop/cop/qa/ambiguous_page_object_name.rb @@ -16,7 +16,7 @@ module RuboCop # # good # Page::Object.perform do |object| do ... # Page::Another.perform { |another| ... } - class AmbiguousPageObjectName < RuboCop::Cop::Cop + class AmbiguousPageObjectName < RuboCop::Cop::Base include QAHelpers MESSAGE = "Don't use 'page' as a name for a Page Object. Use `%s` instead." diff --git a/rubocop/cop/qa/element_with_pattern.rb b/rubocop/cop/qa/element_with_pattern.rb index d0a42497960..387ca3eb517 100644 --- a/rubocop/cop/qa/element_with_pattern.rb +++ b/rubocop/cop/qa/element_with_pattern.rb @@ -16,7 +16,7 @@ module RuboCop # # good # element :some_element # element :some_element, required: true - class ElementWithPattern < RuboCop::Cop::Cop + class ElementWithPattern < RuboCop::Cop::Base include QAHelpers MESSAGE = "Don't use a pattern for element, create a corresponding `%s` instead." diff --git a/rubocop/cop/qa/selector_usage.rb b/rubocop/cop/qa/selector_usage.rb index 568b1c30851..d615bd2926c 100644 --- a/rubocop/cop/qa/selector_usage.rb +++ b/rubocop/cop/qa/selector_usage.rb @@ -16,7 +16,7 @@ module RuboCop # # good # find('[data-testid="the_selector"]') # find('#selector') - class SelectorUsage < RuboCop::Cop::Cop + class SelectorUsage < RuboCop::Cop::Base include QAHelpers include CodeReuseHelpers diff --git a/rubocop/cop/rspec/any_instance_of.rb b/rubocop/cop/rspec/any_instance_of.rb index a939af36c13..e1cacfebfd3 100644 --- a/rubocop/cop/rspec/any_instance_of.rb +++ b/rubocop/cop/rspec/any_instance_of.rb @@ -24,7 +24,9 @@ module RuboCop # expect(instance).to receive(:invalidate_issue_cache_counts) # end # - class AnyInstanceOf < RuboCop::Cop::Cop + class AnyInstanceOf < RuboCop::Cop::Base + extend RuboCop::Cop::AutoCorrector + MESSAGE_EXPECT = 'Do not use `expect_any_instance_of` method, use `expect_next_instance_of` instead.' MESSAGE_ALLOW = 'Do not use `allow_any_instance_of` method, use `allow_next_instance_of` instead.' @@ -37,22 +39,19 @@ module RuboCop def on_send(node) if expect_any_instance_of?(node) - add_offense(node, location: :expression, message: MESSAGE_EXPECT) + add_offense(node, message: MESSAGE_EXPECT) do |corrector| + corrector.replace( + node.loc.expression, + replacement_any_instance_of(node, 'expect') + ) + end elsif allow_any_instance_of?(node) - add_offense(node, location: :expression, message: MESSAGE_ALLOW) - end - end - - def autocorrect(node) - replacement = - if expect_any_instance_of?(node) - replacement_any_instance_of(node, 'expect') - elsif allow_any_instance_of?(node) - replacement_any_instance_of(node, 'allow') + add_offense(node, message: MESSAGE_ALLOW) do |corrector| + corrector.replace( + node.loc.expression, + replacement_any_instance_of(node, 'allow') + ) end - - lambda do |corrector| - corrector.replace(node.loc.expression, replacement) end end diff --git a/rubocop/cop/rspec/be_success_matcher.rb b/rubocop/cop/rspec/be_success_matcher.rb index dce9604b3d7..5a011845075 100644 --- a/rubocop/cop/rspec/be_success_matcher.rb +++ b/rubocop/cop/rspec/be_success_matcher.rb @@ -21,7 +21,9 @@ module RuboCop # # it { is_expected.to be_successful } # - class BeSuccessMatcher < RuboCop::Cop::Cop + class BeSuccessMatcher < RuboCop::Cop::Base + extend RuboCop::Cop::AutoCorrector + MESSAGE = 'Do not use deprecated `success?` method, use `successful?` instead.' def_node_search :expect_to_be_success?, <<~PATTERN @@ -39,11 +41,7 @@ module RuboCop def on_send(node) return unless be_success_usage?(node) - add_offense(node, location: :expression, message: MESSAGE) - end - - def autocorrect(node) - lambda do |corrector| + add_offense(node, message: MESSAGE) do |corrector| corrector.insert_after(node.loc.expression, 'ful') end end diff --git a/rubocop/cop/rspec/env_assignment.rb b/rubocop/cop/rspec/env_assignment.rb index e3075e7bd90..add7897c624 100644 --- a/rubocop/cop/rspec/env_assignment.rb +++ b/rubocop/cop/rspec/env_assignment.rb @@ -16,7 +16,9 @@ module RuboCop # before do # stub_env('FOO', 'bar') # end - class EnvAssignment < RuboCop::Cop::Cop + class EnvAssignment < RuboCop::Cop::Base + extend RuboCop::Cop::AutoCorrector + MESSAGE = "Don't assign to ENV, use `stub_env` instead." def_node_search :env_assignment?, <<~PATTERN @@ -28,11 +30,7 @@ module RuboCop def on_send(node) return unless env_assignment?(node) - add_offense(node, location: :expression, message: MESSAGE) - end - - def autocorrect(node) - lambda do |corrector| + add_offense(node, message: MESSAGE) do |corrector| corrector.replace(node.loc.expression, stub_env(env_key(node), env_value(node))) end end diff --git a/rubocop/cop/rspec/expect_gitlab_tracking.rb b/rubocop/cop/rspec/expect_gitlab_tracking.rb index e3f790f851c..4f92980baa4 100644 --- a/rubocop/cop/rspec/expect_gitlab_tracking.rb +++ b/rubocop/cop/rspec/expect_gitlab_tracking.rb @@ -29,7 +29,7 @@ module RuboCop # it 'does not expect a snowplow event', :snowplow do # expect_no_snowplow_event # end - class ExpectGitlabTracking < RuboCop::Cop::Cop + class ExpectGitlabTracking < RuboCop::Cop::Base MSG = 'Do not expect directly on `Gitlab::Tracking#event`, add the `snowplow` annotation and use ' \ '`expect_snowplow_event` instead. ' \ 'See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#test-snowplow-events' diff --git a/rubocop/cop/rspec/factories_in_migration_specs.rb b/rubocop/cop/rspec/factories_in_migration_specs.rb index f29bbf68cdc..6dde3d4524c 100644 --- a/rubocop/cop/rspec/factories_in_migration_specs.rb +++ b/rubocop/cop/rspec/factories_in_migration_specs.rb @@ -13,7 +13,7 @@ module RuboCop # # good # let(:users) { table(:users) } # let(:user) { users.create!(name: 'User 1', username: 'user1') } - class FactoriesInMigrationSpecs < RuboCop::Cop::Cop + class FactoriesInMigrationSpecs < RuboCop::Cop::Base MESSAGE = "Don't use FactoryBot.%s in migration specs, use `table` instead." FORBIDDEN_METHODS = %i[build build_list create create_list attributes_for].freeze @@ -29,7 +29,7 @@ module RuboCop method = node.children[1] - add_offense(node, location: :expression, message: MESSAGE % method) + add_offense(node, message: MESSAGE % method) end end end diff --git a/rubocop/cop/rspec/factory_bot/inline_association.rb b/rubocop/cop/rspec/factory_bot/inline_association.rb index 1c2b8b55b46..ccc6364fb73 100644 --- a/rubocop/cop/rspec/factory_bot/inline_association.rb +++ b/rubocop/cop/rspec/factory_bot/inline_association.rb @@ -43,7 +43,9 @@ module RuboCop # # creator_id { create(:user).id } # - class InlineAssociation < RuboCop::Cop::Cop + class InlineAssociation < RuboCop::Cop::Base + extend RuboCop::Cop::AutoCorrector + MSG = 'Prefer inline `association` over `%{type}`. ' \ 'See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#factories' @@ -81,11 +83,7 @@ module RuboCop return if chained_call?(node.parent) return unless inside_assocation_definition?(node) - add_offense(node, message: format(MSG, type: type)) - end - - def autocorrect(node) - lambda do |corrector| + add_offense(node, message: format(MSG, type: type)) do |corrector| receiver, type = create_or_build(node) receiver = "#{receiver.source}." if receiver expression = "#{receiver}#{type}" diff --git a/rubocop/cop/rspec/have_gitlab_http_status.rb b/rubocop/cop/rspec/have_gitlab_http_status.rb index d61fb9f2368..86ece72b4f5 100644 --- a/rubocop/cop/rspec/have_gitlab_http_status.rb +++ b/rubocop/cop/rspec/have_gitlab_http_status.rb @@ -22,7 +22,9 @@ module RuboCop # expect(response).to have_gitlab_http_status(:ok) # expect(response).not_to have_gitlab_http_status(:ok) # - class HaveGitlabHttpStatus < RuboCop::Cop::Cop + class HaveGitlabHttpStatus < RuboCop::Cop::Base + extend RuboCop::Cop::AutoCorrector + CODE_TO_SYMBOL = Rack::Utils::SYMBOL_TO_STATUS_CODE.invert MSG_MATCHER_NAME = @@ -66,13 +68,6 @@ module RuboCop offense_for_matcher(node) || offense_for_response_status(node) end - def autocorrect(node) - lambda do |corrector| - replacement = replace_matcher(node) || replace_response_status(node) - corrector.replace(node.source_range, replacement) - end - end - private def offense_for_matcher(node) @@ -85,13 +80,20 @@ module RuboCop return if offenses.empty? - add_offense(node, message: message_for(*offenses)) + add_offense(node, message: message_for(*offenses), &corrector(node)) end def offense_for_response_status(node) return unless response_status_eq?(node) - add_offense(node, message: message_for(MSG_RESPONSE_STATUS)) + add_offense(node, message: message_for(MSG_RESPONSE_STATUS), &corrector(node)) + end + + def corrector(node) + lambda do |corrector| + replacement = replace_matcher(node) || replace_response_status(node) + corrector.replace(node.source_range, replacement) + end end def replace_matcher(node) diff --git a/rubocop/cop/rspec/httparty_basic_auth.rb b/rubocop/cop/rspec/httparty_basic_auth.rb index c6b52ac9781..1e0f7ae7af0 100644 --- a/rubocop/cop/rspec/httparty_basic_auth.rb +++ b/rubocop/cop/rspec/httparty_basic_auth.rb @@ -12,7 +12,9 @@ module RuboCop # # # good # HTTParty.get(url, basic_auth: { username: 'foo' }) - class HTTPartyBasicAuth < RuboCop::Cop::Cop + class HTTPartyBasicAuth < RuboCop::Cop::Base + extend RuboCop::Cop::AutoCorrector + MESSAGE = "`basic_auth: { user: ... }` does not work - replace `user:` with `username:`" RESTRICT_ON_SEND = %i(get put post delete).freeze @@ -35,12 +37,8 @@ module RuboCop def on_send(node) return unless m = httparty_basic_auth?(node) - add_offense(m, location: :expression, message: MESSAGE) - end - - def autocorrect(node) - lambda do |corrector| - corrector.replace(node.loc.expression, 'username') + add_offense(m, message: MESSAGE) do |corrector| + corrector.replace(m, 'username') end end end diff --git a/rubocop/cop/rspec/modify_sidekiq_middleware.rb b/rubocop/cop/rspec/modify_sidekiq_middleware.rb index c38f074eb3a..78e3ba223b0 100644 --- a/rubocop/cop/rspec/modify_sidekiq_middleware.rb +++ b/rubocop/cop/rspec/modify_sidekiq_middleware.rb @@ -20,7 +20,9 @@ module RuboCop # end # # - class ModifySidekiqMiddleware < RuboCop::Cop::Cop + class ModifySidekiqMiddleware < RuboCop::Cop::Base + extend RuboCop::Cop::AutoCorrector + MSG = <<~MSG Don't modify global sidekiq middleware, use the `#with_sidekiq_server_middleware` helper instead @@ -35,11 +37,7 @@ module RuboCop def on_send(node) return unless modifies_sidekiq_middleware?(node) - add_offense(node, location: :expression) - end - - def autocorrect(node) - -> (corrector) do + add_offense(node) do |corrector| corrector.replace(node.loc.expression, 'with_sidekiq_server_middleware') end diff --git a/rubocop/cop/rspec/timecop_freeze.rb b/rubocop/cop/rspec/timecop_freeze.rb index 508b5df7c7f..70e37ecfa55 100644 --- a/rubocop/cop/rspec/timecop_freeze.rb +++ b/rubocop/cop/rspec/timecop_freeze.rb @@ -13,7 +13,9 @@ module RuboCop # # good # freeze_time(Time.current) { example.run } # - class TimecopFreeze < RuboCop::Cop::Cop + class TimecopFreeze < RuboCop::Cop::Base + extend RuboCop::Cop::AutoCorrector + include MatchRange MESSAGE = 'Do not use `Timecop.freeze`, use `freeze_time` instead. ' \ 'See https://gitlab.com/gitlab-org/gitlab/-/issues/214432 for more info.' @@ -25,11 +27,7 @@ module RuboCop def on_send(node) return unless timecop_freeze?(node) - add_offense(node, location: :expression, message: MESSAGE) - end - - def autocorrect(node) - -> (corrector) do + add_offense(node, message: MESSAGE) do |corrector| each_match_range(node.source_range, /^(Timecop\.freeze)/) do |match_range| corrector.replace(match_range, 'freeze_time') end diff --git a/rubocop/cop/rspec/timecop_travel.rb b/rubocop/cop/rspec/timecop_travel.rb index e5416953af7..586567fa0cd 100644 --- a/rubocop/cop/rspec/timecop_travel.rb +++ b/rubocop/cop/rspec/timecop_travel.rb @@ -13,7 +13,9 @@ module RuboCop # # good # travel_to(1.day.ago) { create(:issue) } # - class TimecopTravel < RuboCop::Cop::Cop + class TimecopTravel < RuboCop::Cop::Base + extend RuboCop::Cop::AutoCorrector + include MatchRange MESSAGE = 'Do not use `Timecop.travel`, use `travel_to` instead. ' \ 'See https://gitlab.com/gitlab-org/gitlab/-/issues/214432 for more info.' @@ -25,11 +27,7 @@ module RuboCop def on_send(node) return unless timecop_travel?(node) - add_offense(node, location: :expression, message: MESSAGE) - end - - def autocorrect(node) - -> (corrector) do + add_offense(node, message: MESSAGE) do |corrector| each_match_range(node.source_range, /^(Timecop\.travel)/) do |match_range| corrector.replace(match_range, 'travel_to') end diff --git a/rubocop/cop/rspec/web_mock_enable.rb b/rubocop/cop/rspec/web_mock_enable.rb index bcf7f95dbbd..0bef16a16b0 100644 --- a/rubocop/cop/rspec/web_mock_enable.rb +++ b/rubocop/cop/rspec/web_mock_enable.rb @@ -3,7 +3,9 @@ module RuboCop module Cop module RSpec - class WebMockEnable < RuboCop::Cop::Cop + class WebMockEnable < RuboCop::Cop::Base + extend RuboCop::Cop::AutoCorrector + # This cop checks for `WebMock.disable_net_connect!` usage in specs and # replaces it with `webmock_enable!` # @@ -24,13 +26,9 @@ module RuboCop def on_send(node) if webmock_disable_net_connect?(node) - add_offense(node, location: :expression, message: MESSAGE) - end - end - - def autocorrect(node) - lambda do |corrector| - corrector.replace(node, 'webmock_enable!') + add_offense(node, message: MESSAGE) do |corrector| + corrector.replace(node, 'webmock_enable!') + end end end end diff --git a/rubocop/cop/ruby_interpolation_in_translation.rb b/rubocop/cop/ruby_interpolation_in_translation.rb index c431b4a1977..fec550bf7c6 100644 --- a/rubocop/cop/ruby_interpolation_in_translation.rb +++ b/rubocop/cop/ruby_interpolation_in_translation.rb @@ -2,7 +2,7 @@ module RuboCop module Cop - class RubyInterpolationInTranslation < RuboCop::Cop::Cop + class RubyInterpolationInTranslation < RuboCop::Cop::Base MSG = "Don't use ruby interpolation \#{} inside translated strings, instead use \%{}" TRANSLATION_METHODS = ':_ :s_ :N_ :n_' diff --git a/rubocop/cop/safe_params.rb b/rubocop/cop/safe_params.rb index 2720732c161..78c40ee548e 100644 --- a/rubocop/cop/safe_params.rb +++ b/rubocop/cop/safe_params.rb @@ -2,7 +2,7 @@ module RuboCop module Cop - class SafeParams < RuboCop::Cop::Cop + class SafeParams < RuboCop::Cop::Base MSG = 'Use `safe_params` instead of `params` in url_for.' METHOD_NAME_PATTERN = :url_for @@ -11,7 +11,7 @@ module RuboCop def on_send(node) return unless method_name(node) == METHOD_NAME_PATTERN - add_offense(node, location: :expression) unless safe_params?(node) + add_offense(node) unless safe_params?(node) end private diff --git a/rubocop/cop/scalability/bulk_perform_with_context.rb b/rubocop/cop/scalability/bulk_perform_with_context.rb index bb944b2ad62..a31c20d149b 100644 --- a/rubocop/cop/scalability/bulk_perform_with_context.rb +++ b/rubocop/cop/scalability/bulk_perform_with_context.rb @@ -6,7 +6,7 @@ require_relative '../../code_reuse_helpers' module RuboCop module Cop module Scalability - class BulkPerformWithContext < RuboCop::Cop::Cop + class BulkPerformWithContext < RuboCop::Cop::Base include RuboCop::MigrationHelpers include RuboCop::CodeReuseHelpers @@ -34,7 +34,7 @@ module RuboCop return unless schedules_in_batch_without_context?(node) return if scheduled_for_background_migration?(node) - add_offense(node, location: :expression) + add_offense(node) end private diff --git a/rubocop/cop/scalability/cron_worker_context.rb b/rubocop/cop/scalability/cron_worker_context.rb index 3cc0d42d7bc..ae8b0b328e2 100644 --- a/rubocop/cop/scalability/cron_worker_context.rb +++ b/rubocop/cop/scalability/cron_worker_context.rb @@ -3,7 +3,7 @@ module RuboCop module Cop module Scalability - class CronWorkerContext < RuboCop::Cop::Cop + class CronWorkerContext < RuboCop::Cop::Base MSG = <<~MSG Manually define an ApplicationContext for cronjob-workers. The context is required to add metadata to our logs. @@ -31,7 +31,7 @@ module RuboCop return if defines_contexts?(node.parent) return if schedules_with_batch_context?(node.parent) - add_offense(node.arguments.first, location: :expression) + add_offense(node.arguments.first) end end end diff --git a/rubocop/cop/scalability/file_uploads.rb b/rubocop/cop/scalability/file_uploads.rb index 83017217e32..3ccb9110e79 100644 --- a/rubocop/cop/scalability/file_uploads.rb +++ b/rubocop/cop/scalability/file_uploads.rb @@ -25,7 +25,7 @@ module RuboCop # optional :file, type: ::API::Validations::Types::WorkhorseFile # end # - class FileUploads < RuboCop::Cop::Cop + class FileUploads < RuboCop::Cop::Base MSG = 'Do not upload files without workhorse acceleration. Please refer to https://docs.gitlab.com/ee/development/uploads.html' def_node_search :file_type_params?, <<~PATTERN @@ -43,7 +43,7 @@ module RuboCop def on_send(node) return unless be_file_param_usage?(node) - add_offense(find_file_param(node), location: :expression) + add_offense(find_file_param(node)) end private diff --git a/rubocop/cop/scalability/idempotent_worker.rb b/rubocop/cop/scalability/idempotent_worker.rb index 7abde54ce7e..88b5d796def 100644 --- a/rubocop/cop/scalability/idempotent_worker.rb +++ b/rubocop/cop/scalability/idempotent_worker.rb @@ -23,7 +23,7 @@ module RuboCop # end # end # - class IdempotentWorker < RuboCop::Cop::Cop + class IdempotentWorker < RuboCop::Cop::Base include CodeReuseHelpers HELP_LINK = 'https://github.com/mperham/sidekiq/wiki/Best-Practices#2-make-your-job-idempotent-and-transactional' @@ -51,7 +51,7 @@ module RuboCop return unless in_worker?(node) return if idempotent?(node) - add_offense(node, location: :expression) + add_offense(node) 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 7a2e7ee00b4..34c3767ad11 100644 --- a/rubocop/cop/sidekiq_load_balancing/worker_data_consistency.rb +++ b/rubocop/cop/sidekiq_load_balancing/worker_data_consistency.rb @@ -23,7 +23,7 @@ module RuboCop # end # end # - class WorkerDataConsistency < RuboCop::Cop::Cop + class WorkerDataConsistency < RuboCop::Cop::Base include CodeReuseHelpers HELP_LINK = 'https://docs.gitlab.com/ee/development/sidekiq_style_guide.html#job-data-consistency-strategies' @@ -57,7 +57,7 @@ module RuboCop return unless in_worker?(node) && application_worker?(node) return if data_consistency_defined?(node) - add_offense(node, location: :expression) + add_offense(node) end end end diff --git a/rubocop/cop/sidekiq_options_queue.rb b/rubocop/cop/sidekiq_options_queue.rb index 2574a229ec2..3eee59e69fd 100644 --- a/rubocop/cop/sidekiq_options_queue.rb +++ b/rubocop/cop/sidekiq_options_queue.rb @@ -3,7 +3,7 @@ module RuboCop module Cop # Cop that prevents manually setting a queue in Sidekiq workers. - class SidekiqOptionsQueue < RuboCop::Cop::Cop + class SidekiqOptionsQueue < RuboCop::Cop::Base MSG = 'Do not manually set a queue; `ApplicationWorker` sets one automatically.' def_node_matcher :sidekiq_options?, <<~PATTERN @@ -16,7 +16,7 @@ module RuboCop node.arguments.first.each_node(:pair) do |pair| key_name = pair.key.children[0] - add_offense(pair, location: :expression) if key_name == :queue + add_offense(pair) if key_name == :queue end end end diff --git a/rubocop/cop/static_translation_definition.rb b/rubocop/cop/static_translation_definition.rb index 121b0c18770..aea4dd6ae34 100644 --- a/rubocop/cop/static_translation_definition.rb +++ b/rubocop/cop/static_translation_definition.rb @@ -51,7 +51,7 @@ module RuboCop # end # end # - class StaticTranslationDefinition < RuboCop::Cop::Cop + class StaticTranslationDefinition < RuboCop::Cop::Base 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 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 3aad089d961..a083318288b 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 @@ -12,7 +12,7 @@ module RuboCop # # bad because pipeline_id points to a large table # distinct_count(Ci::Build, :commit_id) # - class DistinctCountByLargeForeignKey < RuboCop::Cop::Cop + class DistinctCountByLargeForeignKey < RuboCop::Cop::Base MSG = 'Avoid doing `%s` on foreign keys for large tables having above 100 million rows.' def_node_matcher :distinct_count?, <<-PATTERN @@ -25,7 +25,7 @@ module RuboCop next if batch_set_to_false?(method_arguments[2]) next if allowed_foreign_key?(method_arguments[1]) - add_offense(node, location: :selector, message: format(MSG, method_name)) + add_offense(node.loc.selector, message: format(MSG, method_name)) end end diff --git a/rubocop/cop/usage_data/histogram_with_large_table.rb b/rubocop/cop/usage_data/histogram_with_large_table.rb index 961773df55c..35ec568792c 100644 --- a/rubocop/cop/usage_data/histogram_with_large_table.rb +++ b/rubocop/cop/usage_data/histogram_with_large_table.rb @@ -11,7 +11,7 @@ module RuboCop # # bad # histogram(Issue, buckets: 1..100) # histogram(User.active, buckets: 1..100) - class HistogramWithLargeTable < RuboCop::Cop::Cop + class HistogramWithLargeTable < RuboCop::Cop::Base MSG = 'Avoid histogram method on %{model_name}' # Match one level const as Issue, Gitlab @@ -38,9 +38,9 @@ module RuboCop class_name = two_level_matches ? two_level_matches.join('::').to_sym : one_level_matches - if large_table?(class_name) - add_offense(node, location: :expression, message: format(MSG, model_name: class_name)) - end + return unless large_table?(class_name) + + add_offense(node, message: format(MSG, model_name: class_name)) end private diff --git a/rubocop/cop/usage_data/instrumentation_superclass.rb b/rubocop/cop/usage_data/instrumentation_superclass.rb index 2ff2ed47a23..601f2340582 100644 --- a/rubocop/cop/usage_data/instrumentation_superclass.rb +++ b/rubocop/cop/usage_data/instrumentation_superclass.rb @@ -16,7 +16,7 @@ module RuboCop # class CountIssues < BaseMetric # # ... # end - class InstrumentationSuperclass < RuboCop::Cop::Cop + class InstrumentationSuperclass < RuboCop::Cop::Base MSG = "Instrumentation classes should subclass one of the following: %{allowed_classes}." BASE_PATTERN = "(const nil? !#allowed_class?)" diff --git a/rubocop/cop/usage_data/large_table.rb b/rubocop/cop/usage_data/large_table.rb index d9d44f74d26..7d50eebaa83 100644 --- a/rubocop/cop/usage_data/large_table.rb +++ b/rubocop/cop/usage_data/large_table.rb @@ -3,7 +3,7 @@ module RuboCop module Cop module UsageData - class LargeTable < RuboCop::Cop::Cop + class LargeTable < RuboCop::Cop::Base # This cop checks that batch count and distinct_count are used in usage_data.rb files in metrics based on ActiveRecord models. # # @example @@ -56,7 +56,7 @@ module RuboCop counters_used = node.ancestors.any? { |ancestor| allowed_method?(ancestor) } unless counters_used - add_offense(node, location: :expression, message: format(MSG, count_methods: count_methods.join(', '), class_name: class_name)) + add_offense(node, message: format(MSG, count_methods: count_methods.join(', '), class_name: class_name)) end end diff --git a/rubocop/cop/user_admin.rb b/rubocop/cop/user_admin.rb index 3ba0e770ec1..643e620666b 100644 --- a/rubocop/cop/user_admin.rb +++ b/rubocop/cop/user_admin.rb @@ -3,7 +3,7 @@ module RuboCop module Cop # Cop that rejects the usage of `User#admin?` - class UserAdmin < RuboCop::Cop::Cop + class UserAdmin < RuboCop::Cop::Base 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 ' \ @@ -26,7 +26,7 @@ module RuboCop def on_handler(node) return unless admin_call?(node) - add_offense(node, location: :selector) + add_offense(node.loc.selector) end end end |