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

IrreversibleMigration: Ignore methods that are part of another method call #49

Merged
merged 3 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ Sequel/IrreversibleMigration:
Reference: https://www.rubydoc.info/gems/rubocop-sequel/RuboCop/Cop/Sequel/IrreversibleMigration
Enabled: true
VersionAdded: 0.3.5
VersionChanged: 0.3.7
VersionChanged: 0.3.8

Sequel/JSONColumn:
Description: >-
Expand Down
8 changes: 7 additions & 1 deletion lib/rubocop/cop/sequel/irreversible_migration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class IrreversibleMigration < Base
set_column_allow_null
].freeze

MSG = 'Avoid using "%<name>s" inside a "change" block. Use "up" & "down" blocks instead.'
MSG = 'Using "%<name>s" inside a "change" block may cause an irreversible migration. Use "up" & "down" instead.'
PRIMARY_KEY_MSG = 'Avoid using "add_primary_key" with an array argument inside a "change" block.'

def on_block(node)
Expand All @@ -46,6 +46,8 @@ def on_block(node)
def validate_node(node)
return if within_create_table_block?(node)

return if part_of_method_call?(node)

add_offense(node.loc.selector, message: format(MSG, name: node.method_name)) unless valid_change_method?(node)

add_offense(node.loc.selector, message: PRIMARY_KEY_MSG) if invalid_primary_key_method?(node)
Expand All @@ -68,6 +70,10 @@ def within_create_table_block?(node)
ancestor.method_name == :create_table
end
end

def part_of_method_call?(node)
node.each_ancestor(:send).count.positive?
end
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/sequel/version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ module RuboCop
module Sequel
# This module holds the RuboCop Sequel version information.
module Version
STRING = '0.3.7'
STRING = '0.3.8'
end
end
end
46 changes: 46 additions & 0 deletions spec/rubocop/cop/sequel/irreversible_migration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,52 @@
expect(offenses.size).to eq(1)
end
end

describe 'and a method is used within an argument' do
let(:source) do
<<~SOURCE
change do
alter_table(:stores) do
add_column(:products, JSON, null: false, default: Sequel.pg_json({}))
add_constraint(
:only_one_user,
(
Sequel.cast(Sequel.~(user_id: nil), Integer) +
Sequel.cast(Sequel.~(owner_id: nil), Integer)
) => 1,
)
end
end
SOURCE
end

it 'does not register an offense with valid methods' do
offenses = inspect_source_within_migration(source)
expect(offenses).to be_empty
end
end

describe 'and an invalid change method contains another invalid change method as an argument' do
let(:source) do
<<~SOURCE
change do
alter_table(:stores) do
drop_column(:products, JSON, null: false, default: Sequel.pg_json({}))
end
end
SOURCE
end

it 'only registers 1 offense' do
offenses = inspect_source_within_migration(source)
expect(offenses.size).to eq(1)
end

it 'only registers an offense for the parent method' do
offenses = inspect_source_within_migration(source)
expect(offenses.first.message).to include('drop_column')
end
end
end

context 'when inside an up block' do
Expand Down
Loading