Skip to content

Commit

Permalink
Added cop Migration/ChangeTableReferences (#47)
Browse files Browse the repository at this point in the history
  • Loading branch information
Bhacaz authored May 19, 2023
1 parent 7057dc6 commit 2cff4e9
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

# main

* Added cop `Migration/ChangeTableReferences` ([#47](https://github.com/petalmd/rubocop-petal/pull/47))
* Added cop `Migration/AlwaysBulkChangeTable` ([#46](https://github.com/petalmd/rubocop-petal/pull/46))

# v1.0.0 (2023-05-15)
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ bundle exec rake 'new_cop[Rails/MyNewCop]'
```

Have a look to [RuboCop documentation](https://docs.rubocop.org/rubocop/1.23/development.html) to get started with
_node pattern_.
_node pattern_. [Rubocop AST](https://github.com/rubocop/rubocop-ast/blob/master/docs/modules/ROOT/pages/node_pattern.adoc)

## Release

Expand Down
6 changes: 6 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ Migration/AlwaysBulkChangeTable:
Include:
- db/migrate/**

Migration/ChangeTableReferences:
Description: 'Prevent using `t.references` or `t.belongs_to` in a change_table.'
Enabled: true
Include:
- db/migrate/**

Migration/ForeignKeyOption:
Description: 'Specify the foreign key option to create the constraint.'
Enabled: true
Expand Down
49 changes: 49 additions & 0 deletions lib/rubocop/cop/migration/change_table_references.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Migration
# Prevent using `t.references` or `t.belongs_to` in a change_table.
# Internally, `t.references` use multiple `ALTER TABLE` statements.
# Since Rails cannot transform automatically `t.references` inside
# a `change_table bulk: true` we can manually create the equivalent
# `ALTER TABLE` statement using `t.bigint`, `t.index` and `t.foreign_key`.
#
# #bad
# change_table :subscriptions, bulk: true do |t|
# t.references :user, null: false, foreign_key: true
# end
#
# #good
# change_table :subscriptions, bulk: true do |t|
# t.bigint :user_id, null: false
# t.index :user_id
# t.foreign_key :users, column: :user_id
# end
class ChangeTableReferences < Base
MSG = 'Use a combination of `t.bigint`, `t.index` and `t.foreign_key` in a change_table.'

# @!method add_references_in_block?(node)
def_node_search :add_references_in_block?, <<~PATTERN
(send lvar /references|belongs_to/ ...)
PATTERN

# @!method change_table?(node)
def_node_search :change_table?, <<~PATTERN
(send nil? :change_table ...)
PATTERN

def on_block(node)
return unless change_table?(node)

references_node = node.children.detect { |n| add_references_in_block?(n) }
return unless references_node

arguments = references_node.child_nodes[1]
references_methods_range = references_node.source_range.with(end_pos: arguments.source_range.begin_pos - 1)
add_offense(references_methods_range)
end
end
end
end
end
35 changes: 35 additions & 0 deletions spec/rubocop/cop/migration/change_table_references_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Migration::ChangeTableReferences, :config do
it 'registers an offense when using references in a change_table' do
expect_offense(<<~RUBY)
change_table :subscriptions, bulk: true do |t|
t.references :user, null: false, foreign_key: true
^^^^^^^^^^^^ Use a combination of `t.bigint`, `t.index` and `t.foreign_key` in a change_table.
end
RUBY

expect_offense(<<~RUBY)
change_table :subscriptions, bulk: true do |t|
t.belongs_to :user, null: false, foreign_key: true
^^^^^^^^^^^^ Use a combination of `t.bigint`, `t.index` and `t.foreign_key` in a change_table.
end
RUBY
end

it 'does not register an offense when using references in a create_table' do
expect_no_offenses(<<~RUBY)
create_table :subscriptions do |t|
t.belongs_to :user, null: false, foreign_key: true
end
RUBY
end

it 'does not register an offense when not using references in a create_table' do
expect_no_offenses(<<~RUBY)
change_table :subscriptions do |t|
t.index :user_id
end
RUBY
end
end

0 comments on commit 2cff4e9

Please sign in to comment.