diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-12-20 17:22:11 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-12-20 17:22:11 +0300 |
commit | 0c872e02b2c822e3397515ec324051ff540f0cd5 (patch) | |
tree | ce2fb6ce7030e4dad0f4118d21ab6453e5938cdd /rubocop | |
parent | f7e05a6853b12f02911494c4b3fe53d9540d74fc (diff) |
Add latest changes from gitlab-org/gitlab@15-7-stable-eev15.7.0-rc42
Diffstat (limited to 'rubocop')
-rw-r--r-- | rubocop/cop/database/multiple_databases.rb | 1 | ||||
-rw-r--r-- | rubocop/cop/feature_flag_usage.rb | 19 | ||||
-rw-r--r-- | rubocop/cop/gitlab/strong_memoize_attr.rb | 73 | ||||
-rw-r--r-- | rubocop/cop/graphql/descriptions.rb | 25 | ||||
-rw-r--r-- | rubocop/cop/migration/add_column_with_default.rb | 23 | ||||
-rw-r--r-- | rubocop/cop/migration/add_limit_to_text_columns.rb | 6 | ||||
-rw-r--r-- | rubocop/cop/migration/batch_migrations_post_only.rb | 37 | ||||
-rw-r--r-- | rubocop/cop/migration/safer_boolean_column.rb | 4 | ||||
-rw-r--r-- | rubocop/cop/migration/versioned_migration_class.rb | 5 | ||||
-rw-r--r-- | rubocop/cop/rspec/avoid_test_prof.rb | 66 | ||||
-rw-r--r-- | rubocop/cop/rspec/timecop_freeze.rb | 41 | ||||
-rw-r--r-- | rubocop/cop/rspec/timecop_travel.rb | 41 | ||||
-rw-r--r-- | rubocop/migration_helpers.rb | 4 | ||||
-rw-r--r-- | rubocop/rubocop-code_reuse.yml | 1 |
14 files changed, 229 insertions, 117 deletions
diff --git a/rubocop/cop/database/multiple_databases.rb b/rubocop/cop/database/multiple_databases.rb index 33ff8acd4d8..26beecdeba7 100644 --- a/rubocop/cop/database/multiple_databases.rb +++ b/rubocop/cop/database/multiple_databases.rb @@ -20,6 +20,7 @@ module RuboCop ALLOWED_METHODS = %i[ no_touching configurations + logger ].freeze def_node_matcher :active_record_base_method_is_used?, <<~PATTERN diff --git a/rubocop/cop/feature_flag_usage.rb b/rubocop/cop/feature_flag_usage.rb new file mode 100644 index 00000000000..60e82633466 --- /dev/null +++ b/rubocop/cop/feature_flag_usage.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + class FeatureFlagUsage < RuboCop::Cop::Base + MSG = 'Do not use Feature Flags for monkey-patches or Redis code. Use environment variables instead.' + + def_node_matcher :using_feature_flag?, <<~PATTERN + (send (const {nil? | cbase} :Feature) {:enabled? | :disabled?} ...) + PATTERN + + def on_send(node) + return unless using_feature_flag?(node) + + add_offense(node, message: MSG) + end + end + end +end diff --git a/rubocop/cop/gitlab/strong_memoize_attr.rb b/rubocop/cop/gitlab/strong_memoize_attr.rb new file mode 100644 index 00000000000..c98aa4765fa --- /dev/null +++ b/rubocop/cop/gitlab/strong_memoize_attr.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Gitlab + # Prefer using `.strong_memoize_attr()` over `#strong_memoize()`. See + # https://docs.gitlab.com/ee/development/utilities.html/#strongmemoize. + # + # Good: + # + # def memoized_method + # 'This is a memoized method' + # end + # strong_memoize_attr :memoized_method + # + # Bad, can be autocorrected: + # + # def memoized_method + # strong_memoize(:memoized_method) do + # 'This is a memoized method' + # end + # end + # + # Very bad, can't be autocorrected: + # + # def memoized_method + # return unless enabled? + # + # strong_memoize(:memoized_method) do + # 'This is a memoized method' + # end + # end + # + class StrongMemoizeAttr < RuboCop::Cop::Base + extend RuboCop::Cop::AutoCorrector + + MSG = 'Use `strong_memoize_attr`, instead of using `strong_memoize` directly' + + def_node_matcher :strong_memoize?, <<~PATTERN + (block + $(send nil? :strong_memoize + (sym $_) + ) + (args) + $_ + ) + PATTERN + + def on_block(node) + send_node, attr_name, body = strong_memoize?(node) + return unless send_node + + corrector = autocorrect_pure_definitions(node.parent, attr_name, body) if node.parent.def_type? + + add_offense(send_node, &corrector) + end + + private + + def autocorrect_pure_definitions(def_node, attr_name, body) + proc do |corrector| + method_name = def_node.method_name + attr_suffix = ", :#{attr_name}" if attr_name != method_name + replacement = "\n#{indent(def_node)}strong_memoize_attr :#{method_name}#{attr_suffix}" + + corrector.insert_after(def_node, replacement) + corrector.replace(def_node.body, body.source) + end + end + end + end + end +end diff --git a/rubocop/cop/graphql/descriptions.rb b/rubocop/cop/graphql/descriptions.rb index 3c945507699..239f5b966a4 100644 --- a/rubocop/cop/graphql/descriptions.rb +++ b/rubocop/cop/graphql/descriptions.rb @@ -3,6 +3,11 @@ # This cop checks for missing GraphQL descriptions and enforces the description style guide: # https://docs.gitlab.com/ee/development/api_graphql_styleguide.html#description-style-guide # +# @safety +# This cop is unsafe because not all cases of "this" can be substituted with +# "the". This will require a technical writer to assist with the alternative, +# proper grammar that can be used for that particular GraphQL descriptions. +# # @examples # # # bad @@ -49,6 +54,8 @@ module RuboCop 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}" MSG_BAD_START = "`description` strings should not start with \"A...\" or \"The...\". #{MSG_STYLE_GUIDE_LINK}" + MSG_CONTAINS_THIS = "`description` strings should not contain the demonstrative \"this\"."\ + " #{MSG_STYLE_GUIDE_LINK}" def_node_matcher :graphql_describable?, <<~PATTERN (send nil? {:field :argument :value} ...) @@ -82,15 +89,17 @@ module RuboCop MSG_NO_PERIOD elsif bad_start?(description) MSG_BAD_START + elsif contains_demonstrative_this?(description) + MSG_CONTAINS_THIS end return unless message add_offense(node, message: message) do |corrector| - description = locate_description(node) next unless description - corrector.insert_after(before_end_quote(description), '.') + corrector.insert_after(before_end_quote(description), '.') if no_period?(description) + corrector.replace(locate_this(description), 'the') if contains_demonstrative_this?(description) end end @@ -114,6 +123,10 @@ module RuboCop string?(description) && description.value.strip.downcase.start_with?('a ', 'the ') end + def contains_demonstrative_this?(description) + string?(description) && /\bthis\b/.match?(description.value.strip) + end + # Returns true if `description` node is a `:str` (as opposed to a `#copy_field_description` call) def string?(description) description.type == :str @@ -127,6 +140,14 @@ module RuboCop adjust = heredoc_source.index(/\s+\Z/) - heredoc_source.length string.location.heredoc_body.adjust(end_pos: adjust) end + + # Returns a `Parser::Source::Range` of the first `this` encountered + def locate_this(string) + target = 'this' + range = string.heredoc? ? string.location.heredoc_body : string.location.expression + index = range.source.index(target) + range.adjust(begin_pos: index, end_pos: (index + target.length) - range.length) + end end end end diff --git a/rubocop/cop/migration/add_column_with_default.rb b/rubocop/cop/migration/add_column_with_default.rb deleted file mode 100644 index 36603e09263..00000000000 --- a/rubocop/cop/migration/add_column_with_default.rb +++ /dev/null @@ -1,23 +0,0 @@ -# frozen_string_literal: true - -require_relative '../../migration_helpers' - -module RuboCop - module Cop - module Migration - class AddColumnWithDefault < RuboCop::Cop::Base - include MigrationHelpers - - MSG = '`add_column_with_default` is deprecated, use `add_column` instead' - - def on_send(node) - return unless in_migration?(node) - - name = node.children[1] - - add_offense(node.loc.selector) if name == :add_column_with_default - end - end - end - 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 5c71fbbfaa2..2d3b5d5094f 100644 --- a/rubocop/cop/migration/add_limit_to_text_columns.rb +++ b/rubocop/cop/migration/add_limit_to_text_columns.rb @@ -44,11 +44,9 @@ module RuboCop if text_operation_with_limit?(send_node) add_offense(send_node.loc.selector, message: TEXT_LIMIT_ATTRIBUTE_NOT_ALLOWED) if version(node) < TEXT_LIMIT_ATTRIBUTE_ALLOWED_SINCE - else + elsif text_limit_missing?(node, *table_and_attribute_name(send_node)) # 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.loc.selector) - end + add_offense(send_node.loc.selector) end end end diff --git a/rubocop/cop/migration/batch_migrations_post_only.rb b/rubocop/cop/migration/batch_migrations_post_only.rb new file mode 100644 index 00000000000..3a1f3b64d75 --- /dev/null +++ b/rubocop/cop/migration/batch_migrations_post_only.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require_relative '../../migration_helpers' + +module RuboCop + module Cop + module Migration + # Cop that checks that no background batched migration helpers are called by regular migrations. + class BatchMigrationsPostOnly < RuboCop::Cop::Base + include MigrationHelpers + + MSG = "This method must only be used in post-deployment migrations." + + FORBIDDEN_METHODS = %w[ + ensure_batched_background_migration_is_finished + queue_batched_background_migration + delete_batched_background_migration + finalize_batched_background_migration + ].freeze + + SYMBOLIZED_MATCHER = FORBIDDEN_METHODS.map { |w| ":#{w}" }.join(' | ') + + def_node_matcher :on_forbidden_method, <<~PATTERN + (send nil? {#{SYMBOLIZED_MATCHER}} ...) + PATTERN + + def on_send(node) + on_forbidden_method(node) do + break if in_post_deployment_migration?(node) + + add_offense(node, message: MSG) + end + end + end + end + end +end diff --git a/rubocop/cop/migration/safer_boolean_column.rb b/rubocop/cop/migration/safer_boolean_column.rb index d3d77b16357..c5df21dc94a 100644 --- a/rubocop/cop/migration/safer_boolean_column.rb +++ b/rubocop/cop/migration/safer_boolean_column.rb @@ -21,9 +21,9 @@ module RuboCop class SaferBooleanColumn < RuboCop::Cop::Base include MigrationHelpers - DEFAULT_OFFENSE = 'Boolean columns on the `%s` table should have a default. You may wish to use `add_column_with_default`.' + DEFAULT_OFFENSE = 'Boolean columns on the `%s` table should have a default.' NULL_OFFENSE = 'Boolean columns on the `%s` table should disallow nulls.' - DEFAULT_AND_NULL_OFFENSE = 'Boolean columns on the `%s` table should have a default and should disallow nulls. You may wish to use `add_column_with_default`.' + DEFAULT_AND_NULL_OFFENSE = 'Boolean columns on the `%s` table should have a default and should disallow nulls.' def_node_matcher :add_column?, <<~PATTERN (send nil? :add_column $...) diff --git a/rubocop/cop/migration/versioned_migration_class.rb b/rubocop/cop/migration/versioned_migration_class.rb index 648782f1735..572ddcd1b12 100644 --- a/rubocop/cop/migration/versioned_migration_class.rb +++ b/rubocop/cop/migration/versioned_migration_class.rb @@ -9,9 +9,10 @@ module RuboCop include MigrationHelpers ENFORCED_SINCE = 2021_09_02_00_00_00 + CURRENT_DATABASE_MIGRATION_CLASS = 'Gitlab::Database::Migration[2.1]' - MSG_INHERIT = 'Don\'t inherit from ActiveRecord::Migration but use Gitlab::Database::Migration[1.0] instead. See https://docs.gitlab.com/ee/development/migration_style_guide.html#migration-helpers-and-versioning.' - MSG_INCLUDE = 'Don\'t include migration helper modules directly. Inherit from Gitlab::Database::Migration[1.0] instead. See https://docs.gitlab.com/ee/development/migration_style_guide.html#migration-helpers-and-versioning.' + MSG_INHERIT = 'Don\'t inherit from ActiveRecord::Migration but use Gitlab::Database::Migration[2.1] instead. See https://docs.gitlab.com/ee/development/migration_style_guide.html#migration-helpers-and-versioning.' + MSG_INCLUDE = 'Don\'t include migration helper modules directly. Inherit from Gitlab::Database::Migration[2.1] instead. See https://docs.gitlab.com/ee/development/migration_style_guide.html#migration-helpers-and-versioning.' ACTIVERECORD_MIGRATION_CLASS = 'ActiveRecord::Migration' diff --git a/rubocop/cop/rspec/avoid_test_prof.rb b/rubocop/cop/rspec/avoid_test_prof.rb new file mode 100644 index 00000000000..fb4b162f125 --- /dev/null +++ b/rubocop/cop/rspec/avoid_test_prof.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'rubocop-rspec' + +module RuboCop + module Cop + module RSpec + # This cop checks for the usage of TestProf methods in migration specs. + # + # @example + # + # # bad + # let_it_be(:user) { table(:users).create(username: 'test') } + # let_it_be_with_reload(:user) { table(:users).create(username: 'test') } + # let_it_be_with_refind(:user) { table(:users).create(username: 'test') } + # + # before_all do + # do_something + # end + # + # # good + # let(:user) { table(:users).create(username: 'test') } + # let!(:user) { table(:users).create(username: 'test') } + # + # before(:all) do + # do_something + # end + # + # before do + # do_something + # end + class AvoidTestProf < RuboCop::Cop::Base + MESSAGE = "Prefer %{alternatives} over `%{method}` in migration specs. " \ + 'See ' \ + 'https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#testprof-in-migration-specs' + + LET_ALTERNATIVES = %w[`let` `let!`].freeze + ALTERNATIVES = { + let_it_be: LET_ALTERNATIVES, + let_it_be_with_reload: LET_ALTERNATIVES, + let_it_be_with_refind: LET_ALTERNATIVES, + before_all: %w[`before` `before(:all)`] + }.freeze + + FORBIDDEN_METHODS = ALTERNATIVES.keys.map(&:inspect).join(' ') + RESTRICT_ON_SEND = ALTERNATIVES.keys + + def_node_matcher :forbidden_method_usage, <<~PATTERN + (send nil? ${#{FORBIDDEN_METHODS}} ...) + PATTERN + + def on_send(node) + method = forbidden_method_usage(node) + return unless method + + alternatives = ALTERNATIVES.fetch(method).join(' or ') + + add_offense( + node, + message: format(MESSAGE, method: method, alternatives: alternatives) + ) + end + end + end + end +end diff --git a/rubocop/cop/rspec/timecop_freeze.rb b/rubocop/cop/rspec/timecop_freeze.rb deleted file mode 100644 index b13f5050040..00000000000 --- a/rubocop/cop/rspec/timecop_freeze.rb +++ /dev/null @@ -1,41 +0,0 @@ -# frozen_string_literal: true - -require 'rubocop-rspec' - -module RuboCop - module Cop - module RSpec - # This cop checks for `Timecop.freeze` usage in specs. - # - # @example - # - # # bad - # Timecop.freeze(Time.current) { example.run } - # - # # good - # freeze_time(Time.current) { example.run } - # - 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.' - - def_node_matcher :timecop_freeze?, <<~PATTERN - (send (const nil? :Timecop) :freeze ?_) - PATTERN - - def on_send(node) - return unless timecop_freeze?(node) - - 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 - end - end - end - end - end -end diff --git a/rubocop/cop/rspec/timecop_travel.rb b/rubocop/cop/rspec/timecop_travel.rb deleted file mode 100644 index 03f978be349..00000000000 --- a/rubocop/cop/rspec/timecop_travel.rb +++ /dev/null @@ -1,41 +0,0 @@ -# frozen_string_literal: true - -require 'rubocop-rspec' - -module RuboCop - module Cop - module RSpec - # This cop checks for `Timecop.travel` usage in specs. - # - # @example - # - # # bad - # Timecop.travel(1.day.ago) { create(:issue) } - # - # # good - # travel_to(1.day.ago) { create(:issue) } - # - 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.' - - def_node_matcher :timecop_travel?, <<~PATTERN - (send (const nil? :Timecop) :travel _) - PATTERN - - def on_send(node) - return unless timecop_travel?(node) - - 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 - end - end - end - end - end -end diff --git a/rubocop/migration_helpers.rb b/rubocop/migration_helpers.rb index 22f3931be73..50d7b198931 100644 --- a/rubocop/migration_helpers.rb +++ b/rubocop/migration_helpers.rb @@ -19,7 +19,7 @@ module RuboCop # List of helpers that add new columns, either directly (ADD_COLUMN_METHODS) # or through a create/alter table (TABLE_METHODS) - ADD_COLUMN_METHODS = %i(add_column add_column_with_default change_column_type_concurrently).freeze + ADD_COLUMN_METHODS = %i(add_column change_column_type_concurrently).freeze TABLE_METHODS = %i(create_table create_table_if_not_exists change_table create_table_with_constraints).freeze @@ -66,7 +66,7 @@ module RuboCop def array_column?(node) node.each_descendant(:pair).any? do |pair_node| pair_node.child_nodes[0].value == :array && # Searching for a (pair (sym :array) (true)) node - pair_node.child_nodes[1].type == :true # RuboCop::AST::Node uses symbols for types, even when that is a :true + pair_node.child_nodes[1].type == :true # RuboCop::AST::Node uses symbols for types, even when that is a :true end end # rubocop:enable Lint/BooleanSymbol diff --git a/rubocop/rubocop-code_reuse.yml b/rubocop/rubocop-code_reuse.yml index a3b75117621..6d54f880759 100644 --- a/rubocop/rubocop-code_reuse.yml +++ b/rubocop/rubocop-code_reuse.yml @@ -26,6 +26,7 @@ CodeReuse/ActiveRecord: - lib/banzai/**/*.rb - lib/gitlab/background_migration/**/*.rb - lib/gitlab/cycle_analytics/**/*.rb + - lib/gitlab/counters/**/*.rb - lib/gitlab/database/**/*.rb - lib/gitlab/database_importers/common_metrics/importer.rb - lib/gitlab/import_export/**/*.rb |