From f7cf60845ec0a5274eeef9a44cf188d19d1f1c6a Mon Sep 17 00:00:00 2001
From: AlasdairWallaceMackie
 <73504675+AlasdairWallaceMackie@users.noreply.github.com>
Date: Sat, 12 Oct 2024 12:35:00 -0400
Subject: [PATCH 01/11] Starting point code and spec

---
 .../sequel/irreversible_migration_change.rb   | 49 ++++++++++++++++++
 .../irreversible_migration_change_spec.rb     | 51 +++++++++++++++++++
 2 files changed, 100 insertions(+)
 create mode 100644 lib/rubocop/cop/sequel/irreversible_migration_change.rb
 create mode 100644 spec/rubocop/cop/sequel/irreversible_migration_change_spec.rb

diff --git a/lib/rubocop/cop/sequel/irreversible_migration_change.rb b/lib/rubocop/cop/sequel/irreversible_migration_change.rb
new file mode 100644
index 0000000..1b9ccd1
--- /dev/null
+++ b/lib/rubocop/cop/sequel/irreversible_migration_change.rb
@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Sequel
+      # IrreversibleMigrationChange looks for methods inside a `change` block that cannot be reversed.
+      class IrreversibleMigrationChange < Base
+        # https://sequel.jeremyevans.net/rdoc/files/doc/migration_rdoc.html#label-A+Basic+Migration
+        VALID_CHANGE_METHODS = %w[
+          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(with a symbol, not an array)
+          add_index
+          add_full_text_index
+          add_spatial_index
+          rename_column
+          set_column_allow_null
+        ].freeze
+
+        MSG = 'This method should not be used in a `change` block. Use a `down` block instead.'
+
+        def_node_matcher :change_block_method?, <<~PATTERN
+          blah blah
+        PATTERN
+
+        def on_send(node)
+          change_block_method?(node) do |args|
+            add_offense(node.loc.selector, message: MSG) if offensive?(args)
+          end
+        end
+
+        private
+
+        def offensive?(args)
+          !VALID_CHANGE_METHODS.include?(args)
+        end
+      end
+    end
+  end
+end
diff --git a/spec/rubocop/cop/sequel/irreversible_migration_change_spec.rb b/spec/rubocop/cop/sequel/irreversible_migration_change_spec.rb
new file mode 100644
index 0000000..759f1e7
--- /dev/null
+++ b/spec/rubocop/cop/sequel/irreversible_migration_change_spec.rb
@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Sequel::IrreversibleMigrationChange do
+  subject(:cop) { described_class.new }
+
+  context 'when inside a change block' do
+    let(:offensive_source) do
+      <<~SOURCE
+        change do
+          drop_column(:products, :name)
+          drop_index(:products, :price)
+        end
+      SOURCE
+    end
+
+    let(:valid_source) do
+      <<~SOURCE
+        change do
+          add_column(:products, :name)
+          add_index(:products, :price)
+        end
+      SOURCE
+    end
+
+    it 'registers an offense when there is an invalid method' do
+      offenses = inspect_source(offensive_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.size).to be_empty
+    end
+  end
+
+  context 'when inside an up block' do
+    let(:source) do
+      <<~SOURCE
+        change do
+          add_column(:products, :name)
+          drop_index(:products, :price)
+        end
+      SOURCE
+    end
+
+    it 'does not register an offense with any methods' do
+      offenses = inspect_source(source)
+      expect(offenses.size).to be_empty
+    end
+  end
+end

From de64e87d173d5fd16d698822ea41c741470d92a8 Mon Sep 17 00:00:00 2001
From: AlasdairWallaceMackie
 <73504675+AlasdairWallaceMackie@users.noreply.github.com>
Date: Sat, 12 Oct 2024 15:24:16 -0400
Subject: [PATCH 02/11] Node traversal

---
 lib/rubocop-sequel.rb                         |  1 +
 .../sequel/irreversible_migration_change.rb   | 24 +++++++++----------
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/lib/rubocop-sequel.rb b/lib/rubocop-sequel.rb
index 0ca6e2e..1afb32f 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_change'
 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_change.rb b/lib/rubocop/cop/sequel/irreversible_migration_change.rb
index 1b9ccd1..ae40404 100644
--- a/lib/rubocop/cop/sequel/irreversible_migration_change.rb
+++ b/lib/rubocop/cop/sequel/irreversible_migration_change.rb
@@ -18,7 +18,7 @@ class IrreversibleMigrationChange < Base
           add_column
           add_constraint
           add_foreign_key
-          add_primary_key(with a symbol, not an array)
+          add_primary_key
           add_index
           add_full_text_index
           add_spatial_index
@@ -27,21 +27,21 @@ class IrreversibleMigrationChange < Base
         ].freeze
 
         MSG = 'This method should not be used in a `change` block. Use a `down` block instead.'
+        PRIMARY_KEY_MESSAGE = 'Cannot use this method with an array parameter inside a `change` block. Use a `down` block instead.'
 
-        def_node_matcher :change_block_method?, <<~PATTERN
-          blah blah
-        PATTERN
+        def on_block(node)
+          return unless node.method_name == :change
 
-        def on_send(node)
-          change_block_method?(node) do |args|
-            add_offense(node.loc.selector, message: MSG) if offensive?(args)
-          end
-        end
+          body = node.body
+          return unless body
 
-        private
+          body.each_node(:send) do |node|
+            add_offense(node.loc.selector, message: MSG) unless VALID_CHANGE_METHODS.include?(node.method_name)
 
-        def offensive?(args)
-          !VALID_CHANGE_METHODS.include?(args)
+            return unless node.method_name == :add_primary_key
+
+            add_offense(node.loc.selector, message: PRIMARY_KEY_MESSAGE) if node.argument_list.any? { |arg| arg.is_a? Array }
+          end
         end
       end
     end

From 8edc603a50f366321705430eac79638d5077881d Mon Sep 17 00:00:00 2001
From: AlasdairWallaceMackie
 <73504675+AlasdairWallaceMackie@users.noreply.github.com>
Date: Sat, 12 Oct 2024 15:24:28 -0400
Subject: [PATCH 03/11] Update config YAML

---
 config/default.yml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/config/default.yml b/config/default.yml
index 7701aef..6e87db0 100644
--- a/config/default.yml
+++ b/config/default.yml
@@ -11,6 +11,14 @@ Sequel/ConcurrentIndex:
   VersionAdded: 0.0.1
   VersionChanged: 0.3.3
 
+Sequel/IrreversibleMigrationChange:
+  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/IrreversibleMigrationChange
+  Enabled: true
+  VersionAdded: 0.0.1
+  VersionChanged: 0.3.3
+
 Sequel/JSONColumn:
   Description: >-
     Advocates the use of the `jsonb` column type over `json` or `hstore` for performance

From cc040a3d282104367af5688a0cf18308ad9110f4 Mon Sep 17 00:00:00 2001
From: AlasdairWallaceMackie
 <73504675+AlasdairWallaceMackie@users.noreply.github.com>
Date: Mon, 14 Oct 2024 16:57:07 -0400
Subject: [PATCH 04/11] Code cleanup, additional specs

---
 .../sequel/irreversible_migration_change.rb   | 20 +++++---
 .../irreversible_migration_change_spec.rb     | 47 ++++++++++++++-----
 2 files changed, 49 insertions(+), 18 deletions(-)

diff --git a/lib/rubocop/cop/sequel/irreversible_migration_change.rb b/lib/rubocop/cop/sequel/irreversible_migration_change.rb
index ae40404..7618909 100644
--- a/lib/rubocop/cop/sequel/irreversible_migration_change.rb
+++ b/lib/rubocop/cop/sequel/irreversible_migration_change.rb
@@ -6,7 +6,7 @@ module Sequel
       # IrreversibleMigrationChange looks for methods inside a `change` block that cannot be reversed.
       class IrreversibleMigrationChange < Base
         # https://sequel.jeremyevans.net/rdoc/files/doc/migration_rdoc.html#label-A+Basic+Migration
-        VALID_CHANGE_METHODS = %w[
+        VALID_CHANGE_METHODS = %i[
           create_table
           create_join_table
           create_view
@@ -27,7 +27,7 @@ class IrreversibleMigrationChange < Base
         ].freeze
 
         MSG = 'This method should not be used in a `change` block. Use a `down` block instead.'
-        PRIMARY_KEY_MESSAGE = 'Cannot use this method with an array parameter inside a `change` block. Use a `down` block instead.'
+        PRIMARY_KEY_MSG = 'Cannot use this method with an array parameter inside a `change` block.'
 
         def on_block(node)
           return unless node.method_name == :change
@@ -35,13 +35,19 @@ def on_block(node)
           body = node.body
           return unless body
 
-          body.each_node(:send) do |node|
-            add_offense(node.loc.selector, message: MSG) unless VALID_CHANGE_METHODS.include?(node.method_name)
+          body.each_node(:send) { |child_node| validate_node(child_node) }
+        end
+
+        private
+
+        def validate_node(node)
+          add_offense(node.loc.selector, message: MSG) unless VALID_CHANGE_METHODS.include?(node.method_name)
+
+          return unless node.method_name == :add_primary_key
 
-            return unless node.method_name == :add_primary_key
+          return unless node.arguments.any?(&:array_type?)
 
-            add_offense(node.loc.selector, message: PRIMARY_KEY_MESSAGE) if node.argument_list.any? { |arg| arg.is_a? Array }
-          end
+          add_offense(node.loc.selector, message: PRIMARY_KEY_MSG)
         end
       end
     end
diff --git a/spec/rubocop/cop/sequel/irreversible_migration_change_spec.rb b/spec/rubocop/cop/sequel/irreversible_migration_change_spec.rb
index 759f1e7..357c065 100644
--- a/spec/rubocop/cop/sequel/irreversible_migration_change_spec.rb
+++ b/spec/rubocop/cop/sequel/irreversible_migration_change_spec.rb
@@ -4,11 +4,13 @@
   subject(:cop) { described_class.new }
 
   context 'when inside a change block' do
-    let(:offensive_source) do
+    let(:invalid_source) do
       <<~SOURCE
         change do
-          drop_column(:products, :name)
-          drop_index(:products, :price)
+          alter_table(:stores) do
+            drop_column(:products, :name)
+            drop_index(:products, :price)
+          end
         end
       SOURCE
     end
@@ -16,36 +18,59 @@
     let(:valid_source) do
       <<~SOURCE
         change do
-          add_column(:products, :name)
-          add_index(:products, :price)
+          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(offensive_source)
+      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.size).to be_empty
+      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
-        change do
-          add_column(:products, :name)
-          drop_index(:products, :price)
+        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.size).to be_empty
+      expect(offenses).to be_empty
     end
   end
 end

From ae333344d6e546f9e560734a48c4c2e263351252 Mon Sep 17 00:00:00 2001
From: Alasdair Wallace Mackie
 <73504675+AlasdairWallaceMackie@users.noreply.github.com>
Date: Fri, 18 Oct 2024 16:16:28 -0400
Subject: [PATCH 05/11] Rename class

---
 config/default.yml                                        | 8 ++++----
 lib/rubocop/cop/sequel/irreversible_migration_change.rb   | 8 ++++----
 .../cop/sequel/irreversible_migration_change_spec.rb      | 2 +-
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/config/default.yml b/config/default.yml
index 6e87db0..d2c2396 100644
--- a/config/default.yml
+++ b/config/default.yml
@@ -11,13 +11,13 @@ Sequel/ConcurrentIndex:
   VersionAdded: 0.0.1
   VersionChanged: 0.3.3
 
-Sequel/IrreversibleMigrationChange:
+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/IrreversibleMigrationChange
+  Reference: https://www.rubydoc.info/gems/rubocop-sequel/RuboCop/Cop/Sequel/IrreversibleMigration
   Enabled: true
-  VersionAdded: 0.0.1
-  VersionChanged: 0.3.3
+  VersionAdded: 0.3.4
+  VersionChanged: 0.3.4
 
 Sequel/JSONColumn:
   Description: >-
diff --git a/lib/rubocop/cop/sequel/irreversible_migration_change.rb b/lib/rubocop/cop/sequel/irreversible_migration_change.rb
index 7618909..94a5a73 100644
--- a/lib/rubocop/cop/sequel/irreversible_migration_change.rb
+++ b/lib/rubocop/cop/sequel/irreversible_migration_change.rb
@@ -3,8 +3,8 @@
 module RuboCop
   module Cop
     module Sequel
-      # IrreversibleMigrationChange looks for methods inside a `change` block that cannot be reversed.
-      class IrreversibleMigrationChange < Base
+      # 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
@@ -26,8 +26,8 @@ class IrreversibleMigrationChange < Base
           set_column_allow_null
         ].freeze
 
-        MSG = 'This method should not be used in a `change` block. Use a `down` block instead.'
-        PRIMARY_KEY_MSG = 'Cannot use this method with an array parameter inside a `change` block.'
+        MSG = 'Avoid using this method inside a `change` block. Use a `down` block instead.'
+        PRIMARY_KEY_MSG = 'Avoid using this method with an array argument inside a `change` block.'
 
         def on_block(node)
           return unless node.method_name == :change
diff --git a/spec/rubocop/cop/sequel/irreversible_migration_change_spec.rb b/spec/rubocop/cop/sequel/irreversible_migration_change_spec.rb
index 357c065..3bbb57c 100644
--- a/spec/rubocop/cop/sequel/irreversible_migration_change_spec.rb
+++ b/spec/rubocop/cop/sequel/irreversible_migration_change_spec.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-RSpec.describe RuboCop::Cop::Sequel::IrreversibleMigrationChange do
+RSpec.describe RuboCop::Cop::Sequel::IrreversibleMigration do
   subject(:cop) { described_class.new }
 
   context 'when inside a change block' do

From 902db3b47d3b21d549f659de2cccb77897d3fa56 Mon Sep 17 00:00:00 2001
From: Alasdair Wallace Mackie
 <73504675+AlasdairWallaceMackie@users.noreply.github.com>
Date: Fri, 18 Oct 2024 16:16:47 -0400
Subject: [PATCH 06/11] Rename files

---
 ...irreversible_migration_change.rb => irreversible_migration.rb} | 0
 ..._migration_change_spec.rb => irreversible_migration_change.rb} | 0
 2 files changed, 0 insertions(+), 0 deletions(-)
 rename lib/rubocop/cop/sequel/{irreversible_migration_change.rb => irreversible_migration.rb} (100%)
 rename spec/rubocop/cop/sequel/{irreversible_migration_change_spec.rb => irreversible_migration_change.rb} (100%)

diff --git a/lib/rubocop/cop/sequel/irreversible_migration_change.rb b/lib/rubocop/cop/sequel/irreversible_migration.rb
similarity index 100%
rename from lib/rubocop/cop/sequel/irreversible_migration_change.rb
rename to lib/rubocop/cop/sequel/irreversible_migration.rb
diff --git a/spec/rubocop/cop/sequel/irreversible_migration_change_spec.rb b/spec/rubocop/cop/sequel/irreversible_migration_change.rb
similarity index 100%
rename from spec/rubocop/cop/sequel/irreversible_migration_change_spec.rb
rename to spec/rubocop/cop/sequel/irreversible_migration_change.rb

From 28c00f6e072e3d24d1f8aa30f4a35f8047d2f733 Mon Sep 17 00:00:00 2001
From: Alasdair Wallace Mackie
 <73504675+AlasdairWallaceMackie@users.noreply.github.com>
Date: Fri, 18 Oct 2024 16:18:20 -0400
Subject: [PATCH 07/11] Fix require

---
 lib/rubocop-sequel.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/rubocop-sequel.rb b/lib/rubocop-sequel.rb
index 1afb32f..1c2ed2f 100644
--- a/lib/rubocop-sequel.rb
+++ b/lib/rubocop-sequel.rb
@@ -8,7 +8,7 @@
 RuboCop::Sequel::Inject.defaults!
 
 require 'rubocop/cop/sequel/concurrent_index'
-require 'rubocop/cop/sequel/irreversible_migration_change'
+require 'rubocop/cop/sequel/irreversible_migration'
 require 'rubocop/cop/sequel/json_column'
 require 'rubocop/cop/sequel/migration_name'
 require 'rubocop/cop/sequel/save_changes'

From 7a86094408efdc35946cbf224403991bd562e8c5 Mon Sep 17 00:00:00 2001
From: Alasdair Wallace Mackie
 <73504675+AlasdairWallaceMackie@users.noreply.github.com>
Date: Fri, 18 Oct 2024 16:20:02 -0400
Subject: [PATCH 08/11] Rename file

---
 ...ersible_migration_change.rb => irreversible_migration_spec.rb} | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename spec/rubocop/cop/sequel/{irreversible_migration_change.rb => irreversible_migration_spec.rb} (100%)

diff --git a/spec/rubocop/cop/sequel/irreversible_migration_change.rb b/spec/rubocop/cop/sequel/irreversible_migration_spec.rb
similarity index 100%
rename from spec/rubocop/cop/sequel/irreversible_migration_change.rb
rename to spec/rubocop/cop/sequel/irreversible_migration_spec.rb

From eff740d2cfbf32c4ea19b326aaf708a7b00e4a91 Mon Sep 17 00:00:00 2001
From: Alasdair Wallace Mackie
 <73504675+AlasdairWallaceMackie@users.noreply.github.com>
Date: Fri, 18 Oct 2024 17:02:29 -0400
Subject: [PATCH 09/11] Format MSG string with method name

---
 lib/rubocop/cop/sequel/irreversible_migration.rb | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/lib/rubocop/cop/sequel/irreversible_migration.rb b/lib/rubocop/cop/sequel/irreversible_migration.rb
index 94a5a73..452f8a8 100644
--- a/lib/rubocop/cop/sequel/irreversible_migration.rb
+++ b/lib/rubocop/cop/sequel/irreversible_migration.rb
@@ -26,8 +26,8 @@ class IrreversibleMigration < Base
           set_column_allow_null
         ].freeze
 
-        MSG = 'Avoid using this method inside a `change` block. Use a `down` block instead.'
-        PRIMARY_KEY_MSG = 'Avoid using this method with an array argument inside a `change` block.'
+        MSG = 'Avoid using `%<name>s` inside a `change` block. Use a `down` block 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
@@ -41,9 +41,11 @@ def on_block(node)
         private
 
         def validate_node(node)
-          add_offense(node.loc.selector, message: MSG) unless VALID_CHANGE_METHODS.include?(node.method_name)
+          name = node.method_name
 
-          return unless node.method_name == :add_primary_key
+          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?)
 

From bc97248498d58c791abc2ddb75ece2f6347778ae Mon Sep 17 00:00:00 2001
From: AlasdairWallaceMackie
 <73504675+AlasdairWallaceMackie@users.noreply.github.com>
Date: Sun, 20 Oct 2024 11:00:04 -0400
Subject: [PATCH 10/11] Change `add_runtime_dependency` to `add_dependency`

---
 rubocop-sequel.gemspec | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

From 5001f1e1c42a65218cd4a49524e88c0b01f455ff Mon Sep 17 00:00:00 2001
From: Alasdair Wallace Mackie
 <73504675+AlasdairWallaceMackie@users.noreply.github.com>
Date: Mon, 21 Oct 2024 10:32:38 -0400
Subject: [PATCH 11/11] Small change to `MSG`

---
 lib/rubocop/cop/sequel/irreversible_migration.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/rubocop/cop/sequel/irreversible_migration.rb b/lib/rubocop/cop/sequel/irreversible_migration.rb
index 452f8a8..d3455c1 100644
--- a/lib/rubocop/cop/sequel/irreversible_migration.rb
+++ b/lib/rubocop/cop/sequel/irreversible_migration.rb
@@ -26,7 +26,7 @@ class IrreversibleMigration < Base
           set_column_allow_null
         ].freeze
 
-        MSG = 'Avoid using `%<name>s` inside a `change` block. Use a `down` block instead.'
+        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)