From 82db9ec50ae17c7ae0c1da0dd89d50002201b5f3 Mon Sep 17 00:00:00 2001 From: Jean-Francis Bastien Date: Fri, 16 Jun 2023 21:35:10 -0400 Subject: [PATCH 1/7] Added `Migration/StandaloneAddReference` --- CHANGELOG.md | 2 ++ config/default.yml | 6 ++++ .../cop/migration/standalone_add_reference.rb | 32 +++++++++++++++++++ .../standalone_add_reference_spec.rb | 31 ++++++++++++++++++ 4 files changed, 71 insertions(+) create mode 100644 lib/rubocop/cop/migration/standalone_add_reference.rb create mode 100644 spec/rubocop/cop/migration/standalone_add_reference_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index c79f720..b90ffa8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ # main +* Added cop `Migration/StandaloneAddReference` ([#X](https://github.com/petalmd/rubocop-petal/pull/X)) + # v1.1.2 (2023-05-30) * Fix `Migration/ChangeTableReferences` with multiple nested blocks. ([#51](https://github.com/petalmd/rubocop-petal/pull/51)) diff --git a/config/default.yml b/config/default.yml index 71a09b3..a83ff96 100644 --- a/config/default.yml +++ b/config/default.yml @@ -29,6 +29,12 @@ Migration/SchemaStatementsMethods: Include: - db/migrate/** +Migration/StandaloneAddReference: + Description: 'Prevent using `add_reference/belongs_to` outside of a change_table.' + Enabled: true + Include: + - db/migrate/** + RSpec/AuthenticatedAs: Description: 'Suggest to use authenticated_as instead of legacy api_key.' Enabled: true diff --git a/lib/rubocop/cop/migration/standalone_add_reference.rb b/lib/rubocop/cop/migration/standalone_add_reference.rb new file mode 100644 index 0000000..2261a3c --- /dev/null +++ b/lib/rubocop/cop/migration/standalone_add_reference.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Migration + # Prevent using `add_reference` and `remove_reference` outside of. + # a `change_table` block. `add_reference` create multiple `ALTER TABLE` + # statements. Using `change_table` with `bulk: true` is more efficient. + # + # # bad + # add_reference :products, :user, foreign_key: true + # + # # 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 StandaloneAddReference < Base + MSG = 'Modifying references must be done in a change_table block.' + + RESTRICT_ON_SEND = %i[add_reference belongs_to remove_reference].freeze + + def on_send(node) + reference_method = node.source_range.with(end_pos: node.child_nodes.first.source_range.begin_pos - 1) + + add_offense(reference_method) + end + end + end + end +end diff --git a/spec/rubocop/cop/migration/standalone_add_reference_spec.rb b/spec/rubocop/cop/migration/standalone_add_reference_spec.rb new file mode 100644 index 0000000..53df8cd --- /dev/null +++ b/spec/rubocop/cop/migration/standalone_add_reference_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Migration::StandaloneAddReference, :config do + it 'registers an offense when modifing a references outside a change_table' do + expect_offense(<<~RUBY) + add_reference :products, :user + ^^^^^^^^^^^^^^ Modifying references must be done in a change_table block. + RUBY + + expect_offense(<<~RUBY) + add_reference :products, :user, index: true + ^^^^^^^^^^^^^^ Modifying references must be done in a change_table block. + RUBY + + expect_offense(<<~RUBY) + belongs_to :products, :user + ^^^^^^^^^^ Modifying references must be done in a change_table block. + RUBY + + expect_offense(<<~RUBY) + remove_reference :products, :user + ^^^^^^^^^^^^^^^^^ Modifying references must be done in a change_table block. + RUBY + end + + it 'does not register an offense when not calling modifying references methods' do + expect_no_offenses(<<~RUBY) + add_index :users, :user_id + RUBY + end +end From 53d82c34fa37202243986959f8664b39678ecb61 Mon Sep 17 00:00:00 2001 From: Jean-Francis Bastien Date: Fri, 16 Jun 2023 21:35:34 -0400 Subject: [PATCH 2/7] changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b90ffa8..ae61b1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ # main -* Added cop `Migration/StandaloneAddReference` ([#X](https://github.com/petalmd/rubocop-petal/pull/X)) +* Added cop `Migration/StandaloneAddReference` ([#54](https://github.com/petalmd/rubocop-petal/pull/54)) # v1.1.2 (2023-05-30) From e6b06e13272425328b0388fa3e83d16bbf2b7156 Mon Sep 17 00:00:00 2001 From: Jean-Francis Bastien Date: Fri, 16 Jun 2023 21:36:06 -0400 Subject: [PATCH 3/7] update comment --- lib/rubocop/cop/migration/standalone_add_reference.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rubocop/cop/migration/standalone_add_reference.rb b/lib/rubocop/cop/migration/standalone_add_reference.rb index 2261a3c..c30ccb2 100644 --- a/lib/rubocop/cop/migration/standalone_add_reference.rb +++ b/lib/rubocop/cop/migration/standalone_add_reference.rb @@ -11,7 +11,7 @@ module Migration # add_reference :products, :user, foreign_key: true # # # good - # change_table :subscriptions, bulk: true do |t| + # change_table :products, bulk: true do |t| # t.bigint :user_id, null: false # t.index :user_id # t.foreign_key :users, column: :user_id From 1a77aed9d2b5103f62e47e000c471646f71ff92d Mon Sep 17 00:00:00 2001 From: Jean-Francis Bastien Date: Fri, 16 Jun 2023 21:37:28 -0400 Subject: [PATCH 4/7] spec --- spec/rubocop/cop/migration/standalone_add_reference_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/rubocop/cop/migration/standalone_add_reference_spec.rb b/spec/rubocop/cop/migration/standalone_add_reference_spec.rb index 53df8cd..f34c355 100644 --- a/spec/rubocop/cop/migration/standalone_add_reference_spec.rb +++ b/spec/rubocop/cop/migration/standalone_add_reference_spec.rb @@ -4,12 +4,12 @@ it 'registers an offense when modifing a references outside a change_table' do expect_offense(<<~RUBY) add_reference :products, :user - ^^^^^^^^^^^^^^ Modifying references must be done in a change_table block. + ^^^^^^^^^^^^^ Modifying references must be done in a change_table block. RUBY expect_offense(<<~RUBY) add_reference :products, :user, index: true - ^^^^^^^^^^^^^^ Modifying references must be done in a change_table block. + ^^^^^^^^^^^^^ Modifying references must be done in a change_table block. RUBY expect_offense(<<~RUBY) @@ -19,7 +19,7 @@ expect_offense(<<~RUBY) remove_reference :products, :user - ^^^^^^^^^^^^^^^^^ Modifying references must be done in a change_table block. + ^^^^^^^^^^^^^^^^ Modifying references must be done in a change_table block. RUBY end From 266c6813b9293b0a5f4e61ae6ed771c4534ac3de Mon Sep 17 00:00:00 2001 From: Jean-Francis Bastien Date: Fri, 16 Jun 2023 21:49:36 -0400 Subject: [PATCH 5/7] remove_belongs_to --- lib/rubocop/cop/migration/standalone_add_reference.rb | 2 +- spec/rubocop/cop/migration/standalone_add_reference_spec.rb | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/rubocop/cop/migration/standalone_add_reference.rb b/lib/rubocop/cop/migration/standalone_add_reference.rb index c30ccb2..0f61678 100644 --- a/lib/rubocop/cop/migration/standalone_add_reference.rb +++ b/lib/rubocop/cop/migration/standalone_add_reference.rb @@ -19,7 +19,7 @@ module Migration class StandaloneAddReference < Base MSG = 'Modifying references must be done in a change_table block.' - RESTRICT_ON_SEND = %i[add_reference belongs_to remove_reference].freeze + RESTRICT_ON_SEND = %i[add_reference belongs_to remove_reference remove_belongs_to].freeze def on_send(node) reference_method = node.source_range.with(end_pos: node.child_nodes.first.source_range.begin_pos - 1) diff --git a/spec/rubocop/cop/migration/standalone_add_reference_spec.rb b/spec/rubocop/cop/migration/standalone_add_reference_spec.rb index f34c355..0ca94ad 100644 --- a/spec/rubocop/cop/migration/standalone_add_reference_spec.rb +++ b/spec/rubocop/cop/migration/standalone_add_reference_spec.rb @@ -21,6 +21,11 @@ remove_reference :products, :user ^^^^^^^^^^^^^^^^ Modifying references must be done in a change_table block. RUBY + + expect_offense(<<~RUBY) + remove_belongs_to :products, :user + ^^^^^^^^^^^^^^^^^ Modifying references must be done in a change_table block. + RUBY end it 'does not register an offense when not calling modifying references methods' do From a776557e596168ce527af9a3d3b1389e43e2a16b Mon Sep 17 00:00:00 2001 From: Jean-Francis Bastien Date: Mon, 19 Jun 2023 09:46:27 -0400 Subject: [PATCH 6/7] Comment --- .../cop/migration/standalone_add_reference.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/rubocop/cop/migration/standalone_add_reference.rb b/lib/rubocop/cop/migration/standalone_add_reference.rb index 0f61678..260a09f 100644 --- a/lib/rubocop/cop/migration/standalone_add_reference.rb +++ b/lib/rubocop/cop/migration/standalone_add_reference.rb @@ -3,19 +3,19 @@ module RuboCop module Cop module Migration - # Prevent using `add_reference` and `remove_reference` outside of. - # a `change_table` block. `add_reference` create multiple `ALTER TABLE` + # Prevent using `add_reference` and `remove_reference` outside of + # a `change_table` block. `add_reference` create multiples `ALTER TABLE` # statements. Using `change_table` with `bulk: true` is more efficient. # # # bad # add_reference :products, :user, foreign_key: true # - # # good - # change_table :products, bulk: true do |t| - # t.bigint :user_id, null: false - # t.index :user_id - # t.foreign_key :users, column: :user_id - # end + # # good + # change_table :products, bulk: true do |t| + # t.bigint :user_id, null: false + # t.index :user_id + # t.foreign_key :users, column: :user_id + # end class StandaloneAddReference < Base MSG = 'Modifying references must be done in a change_table block.' From 895ab03cb3f4947a2fcc118fe128ec3f6b4b5d7c Mon Sep 17 00:00:00 2001 From: Jean-Francis Bastien Date: Mon, 19 Jun 2023 11:00:13 -0400 Subject: [PATCH 7/7] add_belongs_to --- lib/rubocop/cop/migration/standalone_add_reference.rb | 2 +- spec/rubocop/cop/migration/standalone_add_reference_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/rubocop/cop/migration/standalone_add_reference.rb b/lib/rubocop/cop/migration/standalone_add_reference.rb index 260a09f..0db089b 100644 --- a/lib/rubocop/cop/migration/standalone_add_reference.rb +++ b/lib/rubocop/cop/migration/standalone_add_reference.rb @@ -19,7 +19,7 @@ module Migration class StandaloneAddReference < Base MSG = 'Modifying references must be done in a change_table block.' - RESTRICT_ON_SEND = %i[add_reference belongs_to remove_reference remove_belongs_to].freeze + RESTRICT_ON_SEND = %i[add_reference add_belongs_to remove_reference remove_belongs_to].freeze def on_send(node) reference_method = node.source_range.with(end_pos: node.child_nodes.first.source_range.begin_pos - 1) diff --git a/spec/rubocop/cop/migration/standalone_add_reference_spec.rb b/spec/rubocop/cop/migration/standalone_add_reference_spec.rb index 0ca94ad..931f614 100644 --- a/spec/rubocop/cop/migration/standalone_add_reference_spec.rb +++ b/spec/rubocop/cop/migration/standalone_add_reference_spec.rb @@ -13,8 +13,8 @@ RUBY expect_offense(<<~RUBY) - belongs_to :products, :user - ^^^^^^^^^^ Modifying references must be done in a change_table block. + add_belongs_to :products, :user + ^^^^^^^^^^^^^^ Modifying references must be done in a change_table block. RUBY expect_offense(<<~RUBY)