Ruby 1.9 compat: fix warnings, shadowed block vars, and unitialized instance vars

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@8481 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb

@@ -33,10 +33,10 @@ module ActiveRecord
 
           if ids.size == 1
             id = ids.first.to_i
-            record = load_target.detect { |record| id == record.id }
+            record = load_target.detect { |r| id == r.id }
             expects_array ? [record] : record
           else
-            load_target.select { |record| ids.include?(record.id) }
+            load_target.select { |r| ids.include?(r.id) }
           end
         else
           conditions = "#{@finder_sql}"
@@ -84,19 +84,19 @@ module ActiveRecord
           else
             columns = @owner.connection.columns(@reflection.options[:join_table], "#{@reflection.options[:join_table]} Columns")
 
-            attributes = columns.inject({}) do |attributes, column|
+            attributes = columns.inject({}) do |attrs, column|
               case column.name
                 when @reflection.primary_key_name
-                  attributes[column.name] = @owner.quoted_id
+                  attrs[column.name] = @owner.quoted_id
                 when @reflection.association_foreign_key
-                  attributes[column.name] = record.quoted_id
+                  attrs[column.name] = record.quoted_id
                 else
-                  if record.attributes.has_key?(column.name)
+                  if record.has_attribute?(column.name)
                     value = @owner.send(:quote_value, record[column.name], column)
-                    attributes[column.name] = value unless value.nil?
+                    attrs[column.name] = value unless value.nil?
                   end
               end
-              attributes
+              attrs
             end
 
             sql =

activerecord/lib/active_record/associations/has_many_association.rb

@@ -41,10 +41,10 @@ module ActiveRecord
 
           if ids.size == 1
             id = ids.first
-            record = load_target.detect { |record| id == record.id }
+            record = load_target.detect { |r| id == r.id }
             expects_array ? [ record ] : record
           else
-            load_target.select { |record| ids.include?(record.id) }
+            load_target.select { |r| ids.include?(r.id) }
           end
         else
           conditions = "#{@finder_sql}"

activerecord/lib/active_record/associations/has_many_through_association.rb

@@ -262,12 +262,31 @@ module ActiveRecord
         end
 
         def conditions
-          @conditions ||= [
-            (interpolate_sql(@reflection.klass.send(:sanitize_sql, @reflection.options[:conditions])) if @reflection.options[:conditions]),
-            (interpolate_sql(@reflection.active_record.send(:sanitize_sql, @reflection.through_reflection.options[:conditions])) if @reflection.through_reflection.options[:conditions]),
-            (interpolate_sql(@reflection.active_record.send(:sanitize_sql, @reflection.source_reflection.options[:conditions])) if @reflection.source_reflection.options[:conditions]),
-            ("#{@reflection.through_reflection.table_name}.#{@reflection.through_reflection.klass.inheritance_column} = #{@reflection.klass.quote_value(@reflection.through_reflection.klass.name.demodulize)}" unless @reflection.through_reflection.klass.descends_from_active_record?)
-          ].compact.collect { |condition| "(#{condition})" }.join(' AND ') unless (!@reflection.options[:conditions] && !@reflection.through_reflection.options[:conditions]  && !@reflection.source_reflection.options[:conditions] && @reflection.through_reflection.klass.descends_from_active_record?)
+          @conditions = build_conditions unless defined?(@conditions)
+          @conditions
+        end
+
+        def build_conditions
+          association_conditions = @reflection.options[:conditions]
+          through_conditions = @reflection.through_reflection.options[:conditions]
+          source_conditions = @reflection.source_reflection.options[:conditions]
+          uses_sti = !@reflection.through_reflection.klass.descends_from_active_record?
+
+          if association_conditions || through_conditions || source_conditions || uses_sti
+            all = []
+
+            [association_conditions, through_conditions, source_conditions].each do |conditions|
+              all << interpolate_sql(sanitize_sql(conditions)) if conditions
+            end
+
+            all << build_sti_condition if uses_sti
+
+            all.map { |sql| "(#{sql})" } * ' AND '
+          end
+        end
+
+        def build_sti_condition
+          "#{@reflection.through_reflection.table_name}.#{@reflection.through_reflection.klass.inheritance_column} = #{@reflection.klass.quote_value(@reflection.through_reflection.klass.name.demodulize)}"
         end
 
         alias_method :sql_conditions, :conditions

activerecord/lib/active_record/attribute_methods.rb

@@ -5,7 +5,7 @@ module ActiveRecord
 
     def self.included(base)
       base.extend ClassMethods
-      base.attribute_method_suffix *DEFAULT_SUFFIXES
+      base.attribute_method_suffix(*DEFAULT_SUFFIXES)
       base.cattr_accessor :attribute_types_cached_by_default, :instance_writer => false
       base.attribute_types_cached_by_default = ATTRIBUTE_TYPES_CACHED_BY_DEFAULT
     end

activerecord/lib/active_record/base.rb

@@ -592,7 +592,7 @@ module ActiveRecord #:nodoc:
       def update(id, attributes)
         if id.is_a?(Array)
           idx = -1
-          id.collect { |id| idx += 1; update(id, attributes[idx]) }
+          id.collect { |one_id| idx += 1; update(one_id, attributes[idx]) }
         else
           object = find(id)
           object.update_attributes(attributes)
@@ -642,7 +642,11 @@ module ActiveRecord #:nodoc:
       #   todos = [1,2,3]
       #   Todo.destroy(todos)
       def destroy(id)
-        id.is_a?(Array) ? id.each { |id| destroy(id) } : find(id).destroy
+        if id.is_a?(Array)
+          id.map { |one_id| destroy(one_id) }
+        else
+          find(id).destroy
+        end
       end
 
       # Updates all records with details given if they match a set of conditions supplied, limits and order can
@@ -1075,9 +1079,9 @@ module ActiveRecord #:nodoc:
 
       # Returns an array of column objects for the table associated with this class.
       def columns
-        unless @columns
+        unless defined?(@columns) && @columns
           @columns = connection.columns(table_name, "#{name} Columns")
-          @columns.each {|column| column.primary = column.name == primary_key}
+          @columns.each { |column| column.primary = column.name == primary_key }
         end
         @columns
       end
@@ -1217,7 +1221,7 @@ module ActiveRecord #:nodoc:
       # Returns whether this class is a base AR class.  If A is a base class and
       # B descends from A, then B.base_class will return B.
       def abstract_class?
-        abstract_class == true
+        defined?(@abstract_class) && @abstract_class == true
       end
 
       private
@@ -1428,7 +1432,7 @@ module ActiveRecord #:nodoc:
           case join
           when Symbol, Hash, Array
             join_dependency = ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new(self, join, nil)
-            sql << " #{join_dependency.join_associations.collect{|join| join.association_join }.join} "
+            sql << " #{join_dependency.join_associations.collect { |assoc| assoc.association_join }.join} "
           else
             sql << " #{join} "
           end
@@ -1962,7 +1966,7 @@ module ActiveRecord #:nodoc:
 
       # Returns true if this object hasn't been saved yet -- that is, a record for the object doesn't exist yet.
       def new_record?
-        @new_record
+        defined?(@new_record) && @new_record
       end
 
       # * No record exists: Creates a new record with values matching those of the object attributes.
@@ -2213,7 +2217,7 @@ module ActiveRecord #:nodoc:
       # Returns +true+ if the record is read only. Records loaded through joins with piggy-back
       # attributes will be marked as read only since they cannot be saved.
       def readonly?
-        @readonly == true
+        defined?(@readonly) && @readonly == true
       end
 
       # Marks this record as read only.
@@ -2334,11 +2338,11 @@ module ActiveRecord #:nodoc:
       # Returns a copy of the attributes hash where all the values have been safely quoted for use in
       # an SQL statement.
       def attributes_with_quotes(include_primary_key = true, include_readonly_attributes = true)
-        quoted = attributes.inject({}) do |quoted, (name, value)|
+        quoted = attributes.inject({}) do |result, (name, value)|
           if column = column_for_attribute(name)
-            quoted[name] = quote_value(value, column) unless !include_primary_key && column.primary
+            result[name] = quote_value(value, column) unless !include_primary_key && column.primary
           end
-          quoted
+          result
         end
         include_readonly_attributes ? quoted : remove_readonly_attributes(quoted)
       end
@@ -2454,9 +2458,9 @@ module ActiveRecord #:nodoc:
       end
 
       def clone_attributes(reader_method = :read_attribute, attributes = {})
-        self.attribute_names.inject(attributes) do |attributes, name|
-          attributes[name] = clone_attribute_value(reader_method, name)
-          attributes
+        self.attribute_names.inject(attributes) do |attrs, name|
+          attrs[name] = clone_attribute_value(reader_method, name)
+          attrs
         end
       end
 

activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb

@@ -71,7 +71,7 @@ module ActiveRecord
       # also be used to "borrow" the connection to do database work unrelated
       # to any of the specific Active Records.
       def connection
-        if @active_connection_name && (conn = active_connections[@active_connection_name])
+        if defined?(@active_connection_name) && (conn = active_connections[@active_connection_name])
           conn
         else
           # retrieve_connection sets the cache key.

activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb

@@ -192,14 +192,14 @@ module ActiveRecord
           end
 
           def fallback_string_to_date(string)
-            new_date *ParseDate.parsedate(string)[0..2]
+            new_date(*ParseDate.parsedate(string)[0..2])
           end
 
           def fallback_string_to_time(string)
             time_hash = Date._parse(string)
             time_hash[:sec_fraction] = microseconds(time_hash)
 
-            new_time *time_hash.values_at(:year, :mon, :mday, :hour, :min, :sec, :sec_fraction)
+            new_time(*time_hash.values_at(:year, :mon, :mday, :hour, :min, :sec, :sec_fraction))
           end
       end
 

activerecord/lib/active_record/connection_adapters/abstract_adapter.rb

@@ -30,6 +30,7 @@ module ActiveRecord
         @connection, @logger = connection, logger
         @runtime = 0
         @last_verification = 0
+        @query_cache_enabled = false
       end
 
       # Returns the human-readable name of the adapter.  Use mixed case - one

activerecord/lib/active_record/migration.rb

@@ -230,7 +230,7 @@ module ActiveRecord
       # recursively. We use @ignore_new_methods as a guard to indicate whether
       # it is safe for the call to proceed.
       def singleton_method_added(sym) #:nodoc:
-        return if @ignore_new_methods
+        return if defined?(@ignore_new_methods) && @ignore_new_methods
 
         begin
           @ignore_new_methods = true
@@ -356,15 +356,14 @@ module ActiveRecord
 
     private
       def migration_classes
-        migrations = migration_files.inject([]) do |migrations, migration_file|
+        classes = migration_files.inject([]) do |migrations, migration_file|
           load(migration_file)
           version, name = migration_version_and_name(migration_file)
           assert_unique_migration_version(migrations, version.to_i)
           migrations << migration_class(name, version.to_i)
-        end
+        end.sort_by(&:version)
 
-        sorted = migrations.sort_by { |m| m.version }
-        down? ? sorted.reverse : sorted
+        down? ? classes.reverse : classes
       end
 
       def assert_unique_migration_version(migrations, version)

activerecord/lib/active_record/transactions.rb

@@ -15,7 +15,7 @@ module ActiveRecord
       end
     end
 
-    # Transactions are protective blocks where SQL statements are only permanent if they can all succeed as one atomic action. 
+    # Transactions are protective blocks where SQL statements are only permanent if they can all succeed as one atomic action.
     # The classic example is a transfer between two accounts where you can only have a deposit if the withdrawal succeeded and
     # vice versa. Transactions enforce the integrity of the database and guard the data against program errors or database break-downs.
     # So basically you should use transaction blocks whenever you have a number of statements that must be executed together or
@@ -116,7 +116,7 @@ module ActiveRecord
     def rollback_active_record_state!
       id_present = has_attribute?(self.class.primary_key)
       previous_id = id
-      previous_new_record = @new_record
+      previous_new_record = new_record?
       yield
     rescue Exception
       @new_record = previous_new_record
@@ -125,7 +125,7 @@ module ActiveRecord
       else
         @attributes.delete(self.class.primary_key)
         @attributes_cache.delete(self.class.primary_key)
-      end  
+      end
       raise
     end
   end

activerecord/test/active_schema_test_mysql.rb

@@ -3,13 +3,16 @@ require 'abstract_unit'
 class ActiveSchemaTest < Test::Unit::TestCase
   def setup
     ActiveRecord::ConnectionAdapters::MysqlAdapter.class_eval do
-      alias_method :real_execute, :execute
+      alias_method :execute_without_stub, :execute
       def execute(sql, name = nil) return sql end
     end
   end
 
   def teardown
-    ActiveRecord::ConnectionAdapters::MysqlAdapter.send(:alias_method, :execute, :real_execute)
+    ActiveRecord::ConnectionAdapters::MysqlAdapter.class_eval do
+      remove_method :execute
+      alias_method :execute, :execute_without_stub
+    end
   end
 
   def test_drop_table

activerecord/test/adapter_test.rb

@@ -76,6 +76,7 @@ class AdapterTest < Test::Unit::TestCase
     assert_equal 'dbo_posts',  @connection.table_alias_for('dbo.posts')
 
     class << @connection
+      remove_method :table_alias_length
       alias_method :table_alias_length, :old_table_alias_length
     end
   end

activerecord/test/associations/callbacks_test.rb

@@ -128,7 +128,7 @@ class AssociationCallbacksTest < Test::Unit::TestCase
     callback_log = ["before_adding<new>", "after_adding<new>"]
     assert_equal callback_log, project.developers_log
     assert project.save
-    assert_equal 1, project.developers_with_callbacks.count
+    assert_equal 1, project.developers_with_callbacks.size
     assert_equal callback_log, project.developers_log
   end
 

activerecord/test/associations/join_model_test.rb

@@ -304,7 +304,7 @@ class AssociationsJoinModelTest < Test::Unit::TestCase
   end
 
   def test_unavailable_through_reflection
-    assert_raises (ActiveRecord::HasManyThroughAssociationNotFoundError) { authors(:david).nothings }
+    assert_raise(ActiveRecord::HasManyThroughAssociationNotFoundError) { authors(:david).nothings }
   end
 
   def test_has_many_through_join_model_with_conditions
@@ -313,10 +313,10 @@ class AssociationsJoinModelTest < Test::Unit::TestCase
   end
 
   def test_has_many_polymorphic
-    assert_raises ActiveRecord::HasManyThroughAssociationPolymorphicError do
+    assert_raise ActiveRecord::HasManyThroughAssociationPolymorphicError do
       assert_equal posts(:welcome, :thinking), tags(:general).taggables
     end
-    assert_raises ActiveRecord::EagerLoadPolymorphicError do
+    assert_raise ActiveRecord::EagerLoadPolymorphicError do
       assert_equal posts(:welcome, :thinking), tags(:general).taggings.find(:all, :include => :taggable)
     end
   end