diff --git a/config/default.yml b/config/default.yml index 7701aef..d2c2396 100644 --- a/config/default.yml +++ b/config/default.yml @@ -11,6 +11,14 @@ Sequel/ConcurrentIndex: VersionAdded: 0.0.1 VersionChanged: 0.3.3 +Sequel/IrreversibleMigration: + Description: >- + Warns against using certain methods inside a migration file's `change` block that would cause the migration to become irreversible. + Reference: https://www.rubydoc.info/gems/rubocop-sequel/RuboCop/Cop/Sequel/IrreversibleMigration + Enabled: true + VersionAdded: 0.3.4 + VersionChanged: 0.3.4 + Sequel/JSONColumn: Description: >- Advocates the use of the `jsonb` column type over `json` or `hstore` for performance diff --git a/lib/rubocop-sequel.rb b/lib/rubocop-sequel.rb index 0ca6e2e..1c2ed2f 100644 --- a/lib/rubocop-sequel.rb +++ b/lib/rubocop-sequel.rb @@ -8,6 +8,7 @@ RuboCop::Sequel::Inject.defaults! require 'rubocop/cop/sequel/concurrent_index' +require 'rubocop/cop/sequel/irreversible_migration' require 'rubocop/cop/sequel/json_column' require 'rubocop/cop/sequel/migration_name' require 'rubocop/cop/sequel/save_changes' diff --git a/lib/rubocop/cop/sequel/irreversible_migration.rb b/lib/rubocop/cop/sequel/irreversible_migration.rb new file mode 100644 index 0000000..d3455c1 --- /dev/null +++ b/lib/rubocop/cop/sequel/irreversible_migration.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Sequel + # IrreversibleMigration looks for methods inside a `change` block that cannot be reversed. + class IrreversibleMigration < Base + # https://sequel.jeremyevans.net/rdoc/files/doc/migration_rdoc.html#label-A+Basic+Migration + VALID_CHANGE_METHODS = %i[ + create_table + create_join_table + create_view + add_column + add_index + rename_column + rename_table + alter_table + add_column + add_constraint + add_foreign_key + add_primary_key + add_index + add_full_text_index + add_spatial_index + rename_column + set_column_allow_null + ].freeze + + MSG = 'Avoid using `%s` inside a `change` block. Use `up` & `down` blocks instead.' + PRIMARY_KEY_MSG = 'Avoid using `add_primary_key` with an array argument inside a `change` block.' + + def on_block(node) + return unless node.method_name == :change + + body = node.body + return unless body + + body.each_node(:send) { |child_node| validate_node(child_node) } + end + + private + + def validate_node(node) + name = node.method_name + + add_offense(node.loc.selector, message: format(MSG, name: name)) unless VALID_CHANGE_METHODS.include?(name) + + return unless name == :add_primary_key + + return unless node.arguments.any?(&:array_type?) + + add_offense(node.loc.selector, message: PRIMARY_KEY_MSG) + end + end + end + end +end diff --git a/rubocop-sequel.gemspec b/rubocop-sequel.gemspec index 04e7317..fad0656 100644 --- a/rubocop-sequel.gemspec +++ b/rubocop-sequel.gemspec @@ -17,5 +17,5 @@ Gem::Specification.new do |gem| gem.required_ruby_version = '>= 2.5' - gem.add_runtime_dependency 'rubocop', '~> 1.0' + gem.add_dependency 'rubocop', '~> 1.0' end diff --git a/spec/rubocop/cop/sequel/irreversible_migration_spec.rb b/spec/rubocop/cop/sequel/irreversible_migration_spec.rb new file mode 100644 index 0000000..3bbb57c --- /dev/null +++ b/spec/rubocop/cop/sequel/irreversible_migration_spec.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Sequel::IrreversibleMigration do + subject(:cop) { described_class.new } + + context 'when inside a change block' do + let(:invalid_source) do + <<~SOURCE + change do + alter_table(:stores) do + drop_column(:products, :name) + drop_index(:products, :price) + end + end + SOURCE + end + + let(:valid_source) do + <<~SOURCE + change do + alter_table(:stores) do + add_primary_key(:id) + add_column(:products, :name) + add_index(:products, :price) + end + end + SOURCE + end + + it 'registers an offense when there is an invalid method' do + offenses = inspect_source(invalid_source) + expect(offenses.size).to eq(2) + end + + it 'does not register an offense with valid methods' do + offenses = inspect_source(valid_source) + expect(offenses).to be_empty + end + + describe 'and an array is passed into `add_primary_key`' do + let(:source) do + <<~SOURCE + change do + alter_table(:stores) do + add_primary_key([:owner_id, :name]) + end + end + SOURCE + end + + it 'registers an offense' do + offenses = inspect_source(source) + expect(offenses.size).to eq(1) + end + end + end + + context 'when inside an up block' do + let(:source) do + <<~SOURCE + up do + alter_table(:stores) do + add_primary_key([:owner_id, :name]) + add_column(:products, :name) + drop_index(:products, :price) + end + end + SOURCE + end + + it 'does not register an offense with any methods' do + offenses = inspect_source(source) + expect(offenses).to be_empty + end + end +end