Ticket #3005: association-duplicate-names-prevent-v3.patch

activerecord/test/associations_test.rb

@@ -258 +258 @@
-    assert_equal a, firm.account
-    assert_equal a, firm.account(true)
-  end
+
+  def test_prevent_naming_association_with_existing_name
+    firm = Firm.new
+    assert_raise ActiveRecord::Reflection::ReflectionNameAlreadyTaken do
+      class << firm
+        has_one :quote
+      end
+    end
+  end
-end
-
-
@@ -741 +750 @@
-    assert_equal 2, firm.clients.length
-    assert firm.clients.include?(companies(:second_client))
-  end
+
+  def test_prevent_naming_association_with_existing_name
+    firm = Firm.new
+    assert_raise ActiveRecord::Reflection::ReflectionNameAlreadyTaken do
+      class << firm
+        has_many :columns
+      end
+    end
+  end
-end
-
-class BelongsToAssociationsTest < Test::Unit::TestCase
@@ -997 +1015 @@
-    # the author id of the post should be the id we set
-    assert_equal post.author_id, author2.id
-  end
-
+
+
+
+
+
+
+
+
+
-end
-
-
@@ -1417 +1443 @@
-      AND developer_id = #{developer.id}
-    end_sql
-  end
+
+  def test_prevent_naming_association_with_existing_name
+    firm = Firm.new
+    assert_raise ActiveRecord::Reflection::ReflectionNameAlreadyTaken do
+      class << firm
+        has_and_belongs_to_many :columns
+      end
+    end
+  end
-end

activerecord/test/aggregations_test.rb

@@ -63 +63 @@
-  def test_gps_inequality
-    assert GpsLocation.new('39x110') != GpsLocation.new('39x111')
-  end
+
+  def test_prevent_naming_aggregation_with_existing_name
+    assert_raise ActiveRecord::Reflection::ReflectionNameAlreadyTaken do
+      class << customers(:david)
+        composed_of :base_class, :class_name => 'Money'
+      end
+    end
+  end
-end

activerecord/lib/active_record/reflection.rb

@@ -1 +1 @@
-module ActiveRecord
-  module Reflection # :nodoc:
+    class ReflectionNameAlreadyTaken < ActiveRecord::ConfigurationError; end
+
-    def self.included(base)
-      base.extend(ClassMethods)
-    end
@@ -11 +13 @@
-    # You can find the interface for the AggregateReflection and AssociationReflection classes in the abstract MacroReflection class.
-    module ClassMethods
-      def create_reflection(macro, name, options, active_record)
+        guard_against_already_used_reflection_name(name, active_record)
-        case macro
-          when :has_many, :belongs_to, :has_one, :has_and_belongs_to_many
-            reflection = AssociationReflection.new(macro, name, options, active_record)
@@ -48 +51 @@
-      def reflect_on_association(association)
-        reflections[association].is_a?(AssociationReflection) ? reflections[association] : nil
-      end
+
+      protected
+      def guard_against_already_used_reflection_name(name, active_record)
+        methods = active_record.instance_methods
+        methods += ActiveRecord::Base.methods
+        methods -= Object.methods
+        methods.uniq!
+        methods -= %w(name type)
+        return unless methods.include?(name.to_s)
+
+        # Raise the exception and say WHERE we found the method (very helpful)
+        raise_reflection_already_taken(active_record, name, :instance_methods)
+        raise_reflection_already_taken(ActiveRecord::Base, name, :methods)
+
+        # We couldn't find it !  Wow, that's bad.  Real bad...
+        raise ReflectionNameAlreadyTaken, "`#{name}' is the name of an existing method in #{active_record.name} (but I can't find where this method is already defined !).  You will have to rename your association."
+      end
+
+      def raise_reflection_already_taken(klass, association_name, target)
+        loop do
+          methods = klass.send(target)
+          methods -= klass.superclass.send(target) if klass.superclass
+          raise ReflectionNameAlreadyTaken, "`#{association_name}' is the name of an existing method in #{active_record.name} (class #{klass.name} [#{target}]).  You will have to rename your association." if methods.include?(association_name)
+          return if klass == Object
+          klass = klass.superclass
+        end
+      end
-    end
-
-

activerecord/lib/active_record/aggregations.rb

@@ -131 +131 @@
-        class_name  = options[:class_name] || name_to_class_name(name)
-        mapping     = options[:mapping] || [ name, name ]
-
+        create_reflection(:composed_of, part_id, options, self)
+
-        reader_method(name, class_name, mapping)
-        writer_method(name, class_name, mapping)
-        
-        create_reflection(:composed_of, part_id, options, self)
-      end
-
-      private