MigrationParser: recognize tables passed via keyword arguments, ignore comments

lib/railbow/migration_parser.rb

@@ -8,6 +8,7 @@ class MigrationParser
     SINGLE_TABLE_METHODS = %w[
       create_table drop_table change_table
       add_column remove_column rename_column change_column
+      change_column_default change_column_null
       add_index remove_index
       add_reference remove_reference
       add_belongs_to remove_belongs_to
@@ -19,7 +20,9 @@ class MigrationParser
       add_foreign_key remove_foreign_key
     ].freeze
 
-    SINGLE_TABLE_PATTERN = /\b(?:#{SINGLE_TABLE_METHODS.join("|")})\s+[:"](\w+)/
+    # Dual-table methods are included here too: their first argument is always a
+    # table, even when the second is passed as `to_table:` instead of positionally.
+    SINGLE_TABLE_PATTERN = /\b(?:#{(SINGLE_TABLE_METHODS + DUAL_TABLE_METHODS).join("|")})\s+[:"](\w+)/
     DUAL_TABLE_PATTERN = /\b(?:#{DUAL_TABLE_METHODS.join("|")})\s+[:"](\w+)["\s,]+[:"](\w+)/
 
     # SQL keywords followed by a table name
@@ -33,6 +36,17 @@ class MigrationParser
       /\bCREATE\s+TABLE\s+(?:IF\s+NOT\s+EXISTS\s+)?["']?(\w+)["']?/i
     ].freeze
 
+    # Helper/DSL calls that name their target table via a keyword argument, e.g.
+    # `convert_to_monthly_partitions(table: :apples)` or
+    # `remove_foreign_key :accounts, to_table: :owners`. Catches project-level
+    # migration helpers regardless of the method name. Bare `from:`/`to:` are
+    # deliberately not matched — `change_column_default :users, :status,
+    # from: nil, to: "active"` would yield phantom tables.
+    KEYWORD_TABLE_PATTERN = /\b(?:table|to_table|from_table):\s*[:"']?(\w+)/
+
+    # Ruby `#` comments up to end of line, but not `#{}` interpolation.
+    COMMENT_PATTERN = /#(?!\{).*/
+
     # AR class-level query/mutation methods that strongly signal a constant is a model.
     # Kept narrow on purpose — methods like `find`, `all`, `first`, `count`, `create`,
     # `new`, `transaction`, `order`, `includes` collide with stdlib/non-model constants
@@ -60,13 +74,19 @@ def self.extract_tables(filepath)
     def self.extract_tables_from_content(content)
       return [] if content.nil? || content.empty?
 
+      # Comments would otherwise produce phantom tables — an explanatory
+      # "# ALTER TABLE foo ..." or a commented-out create_table reads exactly
+      # like the real thing to the patterns below.
+      content = content.gsub(COMMENT_PATTERN, "")
+
       tables = []
 
       content.scan(SINGLE_TABLE_PATTERN) { |match| tables << match[0] }
       content.scan(DUAL_TABLE_PATTERN) { |match| tables.concat(match) }
       SQL_TABLE_PATTERNS.each do |pattern|
         content.scan(pattern) { |match| tables << match[0] }
       end
+      content.scan(KEYWORD_TABLE_PATTERN) { |match| tables << match[0] }
       tables.uniq!
 
       # Model-based detection is heuristic (a constant that happens to respond

spec/railbow/migration_parser_spec.rb

@@ -172,6 +172,79 @@ def write_migration(content)
     end
   end
 
+  describe "keyword-argument table detection" do
+    it "extracts table from a helper call with table: keyword and symbol value" do
+      content = "convert_to_monthly_partitions(table: :apples, indexes: INDEXES)"
+      expect(described_class.extract_tables_from_content(content)).to eq(["apples"])
+    end
+
+    it "extracts table from table: keyword with string value" do
+      content = 'convert_to_monthly_partitions(table: "apples")'
+      expect(described_class.extract_tables_from_content(content)).to eq(["apples"])
+    end
+
+    it "extracts both tables from remove_foreign_key with to_table: keyword" do
+      content = "remove_foreign_key :accounts, to_table: :owners"
+      expect(described_class.extract_tables_from_content(content)).to contain_exactly("accounts", "owners")
+    end
+
+    it "does not extract from:/to: values as tables" do
+      content = 'change_column_default :users, :status, from: nil, to: "active"'
+      expect(described_class.extract_tables_from_content(content)).to eq(["users"])
+    end
+
+    it "counts keyword tables toward the model-detection guard" do
+      content = <<~RUBY
+        partition_helper(table: :orders)
+        partition_helper(table: :users)
+        Setting.find_each { |s| s.update_column(:x, 1) }
+      RUBY
+      expect(described_class.extract_tables_from_content(content)).to contain_exactly("orders", "users")
+    end
+
+    it "extracts table from a realistic partition-conversion migration" do
+      content = <<~RUBY
+        class PartitionApplesByMonth < ActiveRecord::Migration[8.1]
+          include PartitionConversion
+
+          # Rewrites the table into a partitioned one (ALTER TABLE is not enough:
+          # postgres cannot convert a plain table in place).
+          INDEXES = [
+            [:banana_id, "index_apples_on_banana_id"],
+            [:created_at, "index_apples_on_created_at"]
+          ].freeze
+
+          def up
+            convert_to_monthly_partitions(table: :apples, indexes: INDEXES)
+          end
+        end
+      RUBY
+      expect(described_class.extract_tables_from_content(content)).to eq(["apples"])
+    end
+  end
+
+  describe "comment stripping" do
+    it "ignores SQL keywords inside comments" do
+      content = <<~RUBY
+        # This used to run ALTER TABLE ghosts, kept for reference.
+        def up; end
+      RUBY
+      expect(described_class.extract_tables_from_content(content)).to eq([])
+    end
+
+    it "ignores commented-out DSL calls" do
+      content = "# create_table :ghosts do |t|; end"
+      expect(described_class.extract_tables_from_content(content)).to eq([])
+    end
+
+    it "keeps content after string interpolation on the same line" do
+      content = <<~'RUBY'
+        execute "COMMENT ON TABLE #{tbl} IS 'x'; UPDATE settings SET a = 1"
+      RUBY
+      expect(described_class.extract_tables_from_content(content)).to eq(["settings"])
+    end
+  end
+
   describe "model-based table detection" do
     it "infers table name from ActiveRecord model with find_each" do
       content = <<~RUBY