Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New cop rule: IrreversibleMigration #43

Merged
8 changes: 8 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop-sequel.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
57 changes: 57 additions & 0 deletions lib/rubocop/cop/sequel/irreversible_migration.rb
Original file line number Diff line number Diff line change
@@ -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 `%<name>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
2 changes: 1 addition & 1 deletion rubocop-sequel.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to make this change due to failed CI build

image

end
76 changes: 76 additions & 0 deletions spec/rubocop/cop/sequel/irreversible_migration_spec.rb
Original file line number Diff line number Diff line change
@@ -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