diff options
Diffstat (limited to 'rubocop/cop')
-rw-r--r-- | rubocop/cop/default_scope.rb | 24 | ||||
-rw-r--r-- | rubocop/cop/gitlab/avoid_feature_get.rb | 27 | ||||
-rw-r--r-- | rubocop/cop/gitlab/bulk_insert.rb | 23 | ||||
-rw-r--r-- | rubocop/cop/inject_enterprise_edition_module.rb | 23 | ||||
-rw-r--r-- | rubocop/cop/migration/add_limit_to_text_columns.rb | 3 | ||||
-rw-r--r-- | rubocop/cop/migration/drop_table.rb | 52 | ||||
-rw-r--r-- | rubocop/cop/migration/prevent_strings.rb | 3 | ||||
-rw-r--r-- | rubocop/cop/migration/update_large_table.rb | 49 | ||||
-rw-r--r-- | rubocop/cop/rspec/empty_line_after_shared_example.rb | 64 |
9 files changed, 142 insertions, 126 deletions
diff --git a/rubocop/cop/default_scope.rb b/rubocop/cop/default_scope.rb new file mode 100644 index 00000000000..39f8c8e9ed0 --- /dev/null +++ b/rubocop/cop/default_scope.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + # Cop that blacklists the use of `default_scope`. + class DefaultScope < RuboCop::Cop::Cop + 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 + for more details. + EOF + + def_node_matcher :default_scope?, <<~PATTERN + (send {nil? (const nil? ...)} :default_scope ...) + PATTERN + + def on_send(node) + return unless default_scope?(node) + + add_offense(node, location: :expression) + end + end + end +end diff --git a/rubocop/cop/gitlab/avoid_feature_get.rb b/rubocop/cop/gitlab/avoid_feature_get.rb new file mode 100644 index 00000000000..e36e0b020c0 --- /dev/null +++ b/rubocop/cop/gitlab/avoid_feature_get.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Gitlab + # Cop that blacklists the use of `Feature.get`. + class AvoidFeatureGet < RuboCop::Cop::Cop + MSG = 'Use `Feature.enable/disable` methods instead of `Feature.get`. ' \ + 'See doc/development/testing_guide/best_practices.md#feature-flags-in-tests for more information.' + + def_node_matcher :feature_get?, <<~PATTERN + (send (const nil? :Feature) :get ...) + PATTERN + + def_node_matcher :global_feature_get?, <<~PATTERN + (send (const (cbase) :Feature) :get ...) + PATTERN + + def on_send(node) + return unless feature_get?(node) || global_feature_get?(node) + + add_offense(node, location: :selector) + end + end + end + end +end diff --git a/rubocop/cop/gitlab/bulk_insert.rb b/rubocop/cop/gitlab/bulk_insert.rb new file mode 100644 index 00000000000..c03ffbe0b2a --- /dev/null +++ b/rubocop/cop/gitlab/bulk_insert.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Gitlab + # Cop that disallows the use of `Gitlab::Database.bulk_insert`, in favour of using + # the `BulkInsertSafe` module. + class BulkInsert < RuboCop::Cop::Cop + MSG = 'Use the `BulkInsertSafe` concern, instead of using `Gitlab::Database.bulk_insert`. See https://docs.gitlab.com/ee/development/insert_into_tables_in_batches.html' + + def_node_matcher :raw_union?, <<~PATTERN + (send (const (const nil? :Gitlab) :Database) :bulk_insert ...) + PATTERN + + def on_send(node) + return unless raw_union?(node) + + add_offense(node, location: :expression) + end + end + end + end +end diff --git a/rubocop/cop/inject_enterprise_edition_module.rb b/rubocop/cop/inject_enterprise_edition_module.rb index 7edce94eaee..16e4b647265 100644 --- a/rubocop/cop/inject_enterprise_edition_module.rb +++ b/rubocop/cop/inject_enterprise_edition_module.rb @@ -2,9 +2,8 @@ module RuboCop module Cop - # Cop that blacklists the injecting of EE specific modules anywhere but on - # the last line of a file. Injecting a module in the middle of a file will - # cause merge conflicts, while placing it on the last line will not. + # Cop that blacklists the injecting of EE 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 INVALID_LINE = 'Injecting EE modules must be done on the last line of this file' \ ', outside of any class or module definitions' @@ -17,10 +16,12 @@ module RuboCop CHECK_LINE_METHODS = Set.new(%i[include_if_ee extend_if_ee prepend_if_ee]).freeze - CHECK_LINE_METHODS_REGEXP = Regexp.union(CHECK_LINE_METHODS.map(&:to_s)).freeze - DISALLOW_METHODS = Set.new(%i[include extend prepend]).freeze + COMMENT_OR_EMPTY_LINE = /^\s*(#.*|$)/.freeze + + CHECK_LINE_METHODS_REGEXP = Regexp.union((CHECK_LINE_METHODS + DISALLOW_METHODS).map(&:to_s) + [COMMENT_OR_EMPTY_LINE]).freeze + def ee_const?(node) line = node.location.expression.source_line @@ -44,15 +45,11 @@ module RuboCop line = node.location.line buffer = node.location.expression.source_buffer last_line = buffer.last_line + lines = buffer.source.split("\n") + # We allow multiple includes, extends and prepends as long as they're all at the end. + allowed_line = (line...last_line).all? { |i| CHECK_LINE_METHODS_REGEXP.match?(lines[i - 1]) } - # Parser treats the final newline (if present) as a separate line, - # meaning that a simple `line < last_line` would yield true even though - # the expression is the last line _of code_. - last_line -= 1 if buffer.source.end_with?("\n") - - last_line_content = buffer.source.split("\n")[-1] - - if CHECK_LINE_METHODS_REGEXP.match?(last_line_content) + if allowed_line ignore_node(node) elsif line < last_line add_offense(node, message: INVALID_LINE) diff --git a/rubocop/cop/migration/add_limit_to_text_columns.rb b/rubocop/cop/migration/add_limit_to_text_columns.rb index 15c28bb9266..b578e73f19e 100644 --- a/rubocop/cop/migration/add_limit_to_text_columns.rb +++ b/rubocop/cop/migration/add_limit_to_text_columns.rb @@ -39,6 +39,9 @@ module RuboCop private def text_operation?(node) + # Don't complain about text arrays + return false if array_column?(node) + modifier = node.children[0] migration_method = node.children[1] diff --git a/rubocop/cop/migration/drop_table.rb b/rubocop/cop/migration/drop_table.rb new file mode 100644 index 00000000000..2a0f57c0c13 --- /dev/null +++ b/rubocop/cop/migration/drop_table.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require_relative '../../migration_helpers' + +module RuboCop + module Cop + 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 + include MigrationHelpers + + MSG = <<-MESSAGE.delete("\n").squeeze + `drop_table` in deployment migrations requires downtime. + Drop tables in post-deployment migrations instead. + MESSAGE + + def on_def(node) + return unless in_deployment_migration?(node) + + node.each_descendant(:send) do |send_node| + next unless offensible?(send_node) + + add_offense(send_node, location: :selector) + end + end + + private + + def offensible?(node) + drop_table?(node) || drop_table_in_execute?(node) + end + + def drop_table?(node) + node.children[1] == :drop_table + end + + def drop_table_in_execute?(node) + execute?(node) && drop_table_in_execute_sql?(node) + end + + def execute?(node) + node.children[1] == :execute + end + + def drop_table_in_execute_sql?(node) + node.children[2].to_s.match?(/drop\s+table/i) + end + end + end + end +end diff --git a/rubocop/cop/migration/prevent_strings.rb b/rubocop/cop/migration/prevent_strings.rb index 25c00194698..bfeabd2c78d 100644 --- a/rubocop/cop/migration/prevent_strings.rb +++ b/rubocop/cop/migration/prevent_strings.rb @@ -33,6 +33,9 @@ module RuboCop private def string_operation?(node) + # Don't complain about string arrays + return false if array_column?(node) + modifier = node.children[0] migration_method = node.children[1] diff --git a/rubocop/cop/migration/update_large_table.rb b/rubocop/cop/migration/update_large_table.rb deleted file mode 100644 index e44eadbdb92..00000000000 --- a/rubocop/cop/migration/update_large_table.rb +++ /dev/null @@ -1,49 +0,0 @@ -require_relative '../../migration_helpers' - -module RuboCop - module Cop - module Migration - # This cop checks for `add_column_with_default` on a table that's been - # explicitly blacklisted because of its size. - # - # Even though this helper performs the update in batches to avoid - # downtime, using it with tables with millions of rows still causes a - # significant delay in the deploy process and is best avoided. - # - # See https://gitlab.com/gitlab-com/infrastructure/issues/1602 for more - # information. - class UpdateLargeTable < RuboCop::Cop::Cop - include MigrationHelpers - - MSG = 'Using `%s` on the `%s` table will take a long time to ' \ - 'complete, and should be avoided unless absolutely ' \ - 'necessary'.freeze - - BATCH_UPDATE_METHODS = %w[ - :add_column_with_default - :change_column_type_concurrently - :rename_column_concurrently - :update_column_in_batches - ].join(' ').freeze - - def_node_matcher :batch_update?, <<~PATTERN - (send nil? ${#{BATCH_UPDATE_METHODS}} $(sym ...) ...) - PATTERN - - def on_send(node) - return unless in_migration?(node) - - matches = batch_update?(node) - return unless matches - - update_method = matches.first - table = matches.last.to_a.first - - return unless BLACKLISTED_TABLES.include?(table) - - add_offense(node, location: :expression, message: format(MSG, update_method, table)) - end - end - end - end -end diff --git a/rubocop/cop/rspec/empty_line_after_shared_example.rb b/rubocop/cop/rspec/empty_line_after_shared_example.rb deleted file mode 100644 index 5d09565bd5a..00000000000 --- a/rubocop/cop/rspec/empty_line_after_shared_example.rb +++ /dev/null @@ -1,64 +0,0 @@ -# frozen_string_literal: true - -require 'rubocop/rspec/final_end_location' -require 'rubocop/rspec/blank_line_separation' -require 'rubocop/rspec/language' - -module RuboCop - module Cop - module RSpec - # Checks if there is an empty line after shared example blocks. - # - # @example - # # bad - # RSpec.describe Foo do - # it_behaves_like 'do this first' - # it_behaves_like 'does this' do - # end - # it_behaves_like 'does that' do - # end - # it_behaves_like 'do some more' - # end - # - # # good - # RSpec.describe Foo do - # it_behaves_like 'do this first' - # it_behaves_like 'does this' do - # end - # - # it_behaves_like 'does that' do - # end - # - # it_behaves_like 'do some more' - # end - # - # # fair - it's ok to have non-separated without blocks - # RSpec.describe Foo do - # it_behaves_like 'do this first' - # it_behaves_like 'does this' - # end - # - class EmptyLineAfterSharedExample < RuboCop::Cop::Cop - include RuboCop::RSpec::BlankLineSeparation - include RuboCop::RSpec::Language - - MSG = 'Add an empty line after `%<example>s` block.' - - def_node_matcher :shared_examples, - (SharedGroups::ALL + Includes::ALL).block_pattern - - def on_block(node) - shared_examples(node) do - break if last_child?(node) - - missing_separating_line(node) do |location| - add_offense(node, - location: location, - message: format(MSG, example: node.method_name)) - end - end - end - end - end - end -end |