Making proper_table_name take in options.

The options will specify the prefix and the suffix. Also, I'm moving the
method to be an instance method on the +Migration+ instance. This makes more
sense than being a class method on the +Migrator+ class because the only
place that uses it is on a +Migration+ instance (in a method_missing
hook). The logic for the Migrator shouldn't be doing any work to
calculate the table name, it should be the Migration itself.

Also made some small indentation fixes.

2 files changed, 68 insertions, 19 deletions

diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb
index 19c6f8148b..888e8771ce 100644
--- a/activerecord/lib/active_record/migration.rb
+++ b/activerecord/lib/active_record/migration.rb
@@ -373,23 +373,23 @@ module ActiveRecord
     class << self
       attr_accessor :delegate # :nodoc:
       attr_accessor :disable_ddl_transaction # :nodoc:
-    end
 
-    def self.check_pending!
-      raise ActiveRecord::PendingMigrationError if ActiveRecord::Migrator.needs_migration?
-    end
+      def check_pending!
+        raise ActiveRecord::PendingMigrationError if ActiveRecord::Migrator.needs_migration?
+      end
 
-    def self.method_missing(name, *args, &block) # :nodoc:
-      (delegate || superclass.delegate).send(name, *args, &block)
-    end
+      def method_missing(name, *args, &block) # :nodoc:
+        (delegate || superclass.delegate).send(name, *args, &block)
+      end
 
-    def self.migrate(direction)
-      new.migrate direction
-    end
+      def migrate(direction)
+        new.migrate direction
+      end
 
-    # Disable DDL transactions for this migration.
-    def self.disable_ddl_transaction!
-      @disable_ddl_transaction = true
+      # Disable DDL transactions for this migration.
+      def disable_ddl_transaction!
+        @disable_ddl_transaction = true
+      end
     end
 
     def disable_ddl_transaction # :nodoc:
@@ -617,8 +617,8 @@ module ActiveRecord
       say_with_time "#{method}(#{arg_list})" do
         unless @connection.respond_to? :revert
           unless arguments.empty? || method == :execute
-            arguments[0] = Migrator.proper_table_name(arguments.first)
-            arguments[1] = Migrator.proper_table_name(arguments.second) if method == :rename_table
+            arguments[0] = proper_table_name(arguments.first, table_name_options)
+            arguments[1] = proper_table_name(arguments.second, table_name_options) if method == :rename_table
           end
         end
         return super unless connection.respond_to?(method)
@@ -671,6 +671,17 @@ module ActiveRecord
       copied
     end
 
+    # Finds the correct table name given an Active Record object.
+    # Uses the Active Record object's own table_name, or pre/suffix from the
+    # options passed in.
+    def proper_table_name(name, options = {})
+      if name.respond_to? :table_name
+        name.table_name
+      else
+        "#{options[:table_name_prefix]}#{name}#{options[:table_name_suffix]}"
+      end
+    end
+
     # Determines the version number of the next migration.
     def next_migration_number(number)
       if ActiveRecord::Base.timestamped_migrations
@@ -680,6 +691,13 @@ module ActiveRecord
       end
     end
 
+    def table_name_options(config = ActiveRecord::Base)
+      {
+        table_name_prefix: config.table_name_prefix,
+        table_name_suffix: config.table_name_suffix
+      }
+    end
+
     private
     def execute_block
       if connection.respond_to? :execute_block
@@ -809,12 +827,16 @@ module ActiveRecord
         migrations(migrations_paths).last || NullMigration.new
       end
 
-      def proper_table_name(name)
-        # Use the Active Record objects own table_name, or pre/suffix from ActiveRecord::Base if name is a symbol/string
+      def proper_table_name(name, options = {})
+        ActiveSupport::Deprecation.warn "ActiveRecord::Migrator.proper_table_name is deprecated and will be removed in Rails 4.1. Use the proper_table_name instance method on ActiveRecord::Migration instead"
+        options = {
+          table_name_prefix: ActiveRecord::Base.table_name_prefix,
+          table_name_suffix: ActiveRecord::Base.table_name_suffix
+        }.merge(options)
         if name.respond_to? :table_name
           name.table_name
         else
-          "#{ActiveRecord::Base.table_name_prefix}#{name}#{ActiveRecord::Base.table_name_suffix}"
+          "#{options[:table_name_prefix]}#{name}#{options[:table_name_suffix]}"
         end
       end
 
diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb
index ed080b2995..cbc9e96473 100644
--- a/activerecord/test/cases/migration_test.rb
+++ b/activerecord/test/cases/migration_test.rb
@@ -321,7 +321,7 @@ class MigrationTest < ActiveRecord::TestCase
     assert_equal "schema_migrations", ActiveRecord::Migrator.schema_migrations_table_name
   end
 
-  def test_proper_table_name
+  def test_proper_table_name_on_migrator
     assert_equal "table", ActiveRecord::Migrator.proper_table_name('table')
     assert_equal "table", ActiveRecord::Migrator.proper_table_name(:table)
     assert_equal "reminders", ActiveRecord::Migrator.proper_table_name(Reminder)
@@ -347,6 +347,33 @@ class MigrationTest < ActiveRecord::TestCase
     assert_equal "prefix_table_suffix", ActiveRecord::Migrator.proper_table_name(:table)
   end
 
+  def test_proper_table_name_on_migration
+    migration = ActiveRecord::Migration.new
+    assert_equal "table", migration.proper_table_name('table')
+    assert_equal "table", migration.proper_table_name(:table)
+    assert_equal "reminders", migration.proper_table_name(Reminder)
+    Reminder.reset_table_name
+    assert_equal Reminder.table_name, migration.proper_table_name(Reminder)
+
+    # Use the model's own prefix/suffix if a model is given
+    ActiveRecord::Base.table_name_prefix = "ARprefix_"
+    ActiveRecord::Base.table_name_suffix = "_ARsuffix"
+    Reminder.table_name_prefix = 'prefix_'
+    Reminder.table_name_suffix = '_suffix'
+    Reminder.reset_table_name
+    assert_equal "prefix_reminders_suffix", migration.proper_table_name(Reminder)
+    Reminder.table_name_prefix = ''
+    Reminder.table_name_suffix = ''
+    Reminder.reset_table_name
+
+    # Use AR::Base's prefix/suffix if string or symbol is given
+    ActiveRecord::Base.table_name_prefix = "prefix_"
+    ActiveRecord::Base.table_name_suffix = "_suffix"
+    Reminder.reset_table_name
+    assert_equal "prefix_table_suffix", migration.proper_table_name('table', migration.table_name_options)
+    assert_equal "prefix_table_suffix", migration.proper_table_name(:table, migration.table_name_options)
+  end
+
   def test_rename_table_with_prefix_and_suffix
     assert !Thing.table_exists?
     ActiveRecord::Base.table_name_prefix = 'p_'