diff options
Diffstat (limited to 'rubocop/cop')
| -rw-r--r-- | rubocop/cop/custom_error_class.rb | 64 | ||||
| -rw-r--r-- | rubocop/cop/gem_fetcher.rb | 4 | ||||
| -rw-r--r-- | rubocop/cop/migration/add_column.rb | 4 | ||||
| -rw-r--r-- | rubocop/cop/migration/add_column_with_default.rb | 2 | ||||
| -rw-r--r-- | rubocop/cop/migration/add_concurrent_foreign_key.rb | 2 | ||||
| -rw-r--r-- | rubocop/cop/migration/add_concurrent_index.rb | 34 | ||||
| -rw-r--r-- | rubocop/cop/migration/add_index.rb | 2 |
7 files changed, 105 insertions, 7 deletions
diff --git a/rubocop/cop/custom_error_class.rb b/rubocop/cop/custom_error_class.rb new file mode 100644 index 00000000000..38d93acfe88 --- /dev/null +++ b/rubocop/cop/custom_error_class.rb @@ -0,0 +1,64 @@ +module RuboCop + module Cop + # This cop makes sure that custom error classes, when empty, are declared + # with Class.new. + # + # @example + # # bad + # class FooError < StandardError + # end + # + # # okish + # class FooError < StandardError; end + # + # # good + # FooError = Class.new(StandardError) + class CustomErrorClass < RuboCop::Cop::Cop + MSG = 'Use `Class.new(SuperClass)` to define an empty custom error class.'.freeze + + def on_class(node) + _klass, parent, body = node.children + + return if body + + parent_klass = class_name_from_node(parent) + + return unless parent_klass && parent_klass.to_s.end_with?('Error') + + add_offense(node, :expression) + end + + def autocorrect(node) + klass, parent, _body = node.children + replacement = "#{class_name_from_node(klass)} = Class.new(#{class_name_from_node(parent)})" + + lambda do |corrector| + corrector.replace(node.source_range, replacement) + end + end + + private + + # The nested constant `Foo::Bar::Baz` looks like: + # + # s(:const, + # s(:const, + # s(:const, nil, :Foo), :Bar), :Baz) + # + # So recurse through that to get the name as written in the source. + # + def class_name_from_node(node, suffix = nil) + return unless node&.type == :const + + name = node.children[1].to_s + name = "#{name}::#{suffix}" if suffix + + if node.children[0] + class_name_from_node(node.children[0], name) + else + name + end + end + end + end +end diff --git a/rubocop/cop/gem_fetcher.rb b/rubocop/cop/gem_fetcher.rb index c199f6acab2..e157d8e0791 100644 --- a/rubocop/cop/gem_fetcher.rb +++ b/rubocop/cop/gem_fetcher.rb @@ -4,9 +4,9 @@ module RuboCop # `Gemfile` in order to avoid additional points of failure beyond # rubygems.org. class GemFetcher < RuboCop::Cop::Cop - MSG = 'Do not use gems from git repositories, only use gems from RubyGems.' + MSG = 'Do not use gems from git repositories, only use gems from RubyGems.'.freeze - GIT_KEYS = [:git, :github] + GIT_KEYS = [:git, :github].freeze def on_send(node) return unless gemfile?(node) diff --git a/rubocop/cop/migration/add_column.rb b/rubocop/cop/migration/add_column.rb index 1490fcdd814..d2cf36c454a 100644 --- a/rubocop/cop/migration/add_column.rb +++ b/rubocop/cop/migration/add_column.rb @@ -8,10 +8,10 @@ module RuboCop class AddColumn < RuboCop::Cop::Cop include MigrationHelpers - WHITELISTED_TABLES = [:application_settings] + WHITELISTED_TABLES = [:application_settings].freeze MSG = '`add_column` with a default value requires downtime, ' \ - 'use `add_column_with_default` instead' + 'use `add_column_with_default` instead'.freeze def on_send(node) return unless in_migration?(node) diff --git a/rubocop/cop/migration/add_column_with_default.rb b/rubocop/cop/migration/add_column_with_default.rb index 747d7caf1ef..54a920d4b49 100644 --- a/rubocop/cop/migration/add_column_with_default.rb +++ b/rubocop/cop/migration/add_column_with_default.rb @@ -9,7 +9,7 @@ module RuboCop include MigrationHelpers MSG = '`add_column_with_default` is not reversible so you must manually define ' \ - 'the `up` and `down` methods in your migration class, using `remove_column` in `down`' + 'the `up` and `down` methods in your migration class, using `remove_column` in `down`'.freeze def on_send(node) return unless in_migration?(node) diff --git a/rubocop/cop/migration/add_concurrent_foreign_key.rb b/rubocop/cop/migration/add_concurrent_foreign_key.rb index e40a7087a47..d1fc94d55be 100644 --- a/rubocop/cop/migration/add_concurrent_foreign_key.rb +++ b/rubocop/cop/migration/add_concurrent_foreign_key.rb @@ -8,7 +8,7 @@ module RuboCop class AddConcurrentForeignKey < RuboCop::Cop::Cop include MigrationHelpers - MSG = '`add_foreign_key` requires downtime, use `add_concurrent_foreign_key` instead' + MSG = '`add_foreign_key` requires downtime, use `add_concurrent_foreign_key` instead'.freeze def on_send(node) return unless in_migration?(node) diff --git a/rubocop/cop/migration/add_concurrent_index.rb b/rubocop/cop/migration/add_concurrent_index.rb new file mode 100644 index 00000000000..332fb7dcbd7 --- /dev/null +++ b/rubocop/cop/migration/add_concurrent_index.rb @@ -0,0 +1,34 @@ +require_relative '../../migration_helpers' + +module RuboCop + module Cop + module Migration + # Cop that checks if `add_concurrent_index` is used with `up`/`down` methods + # and not `change`. + class AddConcurrentIndex < RuboCop::Cop::Cop + include MigrationHelpers + + MSG = '`add_concurrent_index` is not reversible so you must manually define ' \ + 'the `up` and `down` methods in your migration class, using `remove_index` in `down`'.freeze + + def on_send(node) + return unless in_migration?(node) + + name = node.children[1] + + return unless name == :add_concurrent_index + + node.each_ancestor(:def) do |def_node| + next unless method_name(def_node) == :change + + add_offense(def_node, :name) + end + end + + def method_name(node) + node.children.first + end + end + end + end +end diff --git a/rubocop/cop/migration/add_index.rb b/rubocop/cop/migration/add_index.rb index 5e6766f6994..fa21a0d6555 100644 --- a/rubocop/cop/migration/add_index.rb +++ b/rubocop/cop/migration/add_index.rb @@ -7,7 +7,7 @@ module RuboCop class AddIndex < RuboCop::Cop::Cop include MigrationHelpers - MSG = '`add_index` requires downtime, use `add_concurrent_index` instead' + MSG = '`add_index` requires downtime, use `add_concurrent_index` instead'.freeze def on_def(node) return unless in_migration?(node) |