Merge pull request #13450 from chancancode/ar_raise_on_conflict

Raise an error if `scope` or `enum` is about to add a conflicting method to the class

Fixed #13389

3 files changed, 121 insertions, 0 deletions

diff --git a/activerecord/test/cases/enum_test.rb b/activerecord/test/cases/enum_test.rb
index 8719f45e76..5cac630a3a 100644
--- a/activerecord/test/cases/enum_test.rb
+++ b/activerecord/test/cases/enum_test.rb
@@ -163,4 +163,63 @@ class EnumTest < ActiveRecord::TestCase
   test "_before_type_cast returns the enum label (required for form fields)" do
     assert_equal "proposed", @book.status_before_type_cast
   end
+
+  test "reserved enum names" do
+    klass = Class.new(ActiveRecord::Base) do
+      self.table_name = "books"
+      enum status: [:proposed, :written, :published]
+    end
+
+    conflicts = [
+      :column,     # generates class method .columns, which conflicts with an AR method
+      :logger,     # generates #logger, which conflicts with an AR method
+      :attributes, # generates #attributes=, which conflicts with an AR method
+    ]
+
+    conflicts.each_with_index do |name, i|
+      assert_raises(ArgumentError, "enum name `#{name}` should not be allowed") do
+        klass.class_eval { enum name => ["value_#{i}"] }
+      end
+    end
+  end
+
+  test "reserved enum values" do
+    klass = Class.new(ActiveRecord::Base) do
+      self.table_name = "books"
+      enum status: [:proposed, :written, :published]
+    end
+
+    conflicts = [
+      :new,      # generates a scope that conflicts with an AR class method
+      :valid,    # generates #valid?, which conflicts with an AR method
+      :save,     # generates #save!, which conflicts with an AR method
+      :proposed, # same value as an existing enum
+    ]
+
+    conflicts.each_with_index do |value, i|
+      assert_raises(ArgumentError, "enum value `#{value}` should not be allowed") do
+        klass.class_eval { enum "status_#{i}" => [value] }
+      end
+    end
+  end
+
+  test "overriding enum method should not raise" do
+    assert_nothing_raised do
+      klass = Class.new(ActiveRecord::Base) do
+        self.table_name = "books"
+
+        def published!
+          super
+          "do publish work..."
+        end
+
+        enum status: [:proposed, :written, :published]
+
+        def written!
+          super
+          "do written work..."
+        end
+      end
+    end
+  end
 end
diff --git a/activerecord/test/cases/finder_respond_to_test.rb b/activerecord/test/cases/finder_respond_to_test.rb
index 3ff22f222f..6ab2657c44 100644
--- a/activerecord/test/cases/finder_respond_to_test.rb
+++ b/activerecord/test/cases/finder_respond_to_test.rb
@@ -5,6 +5,11 @@ class FinderRespondToTest < ActiveRecord::TestCase
 
   fixtures :topics
 
+  def test_should_preserve_normal_respond_to_behaviour_on_base
+    assert_respond_to ActiveRecord::Base, :new
+    assert !ActiveRecord::Base.respond_to?(:find_by_something)
+  end
+
   def test_should_preserve_normal_respond_to_behaviour_and_respond_to_newly_added_method
     class << Topic; self; end.send(:define_method, :method_added_for_finder_respond_to_test) { }
     assert_respond_to Topic, :method_added_for_finder_respond_to_test
diff --git a/activerecord/test/cases/scoping/named_scoping_test.rb b/activerecord/test/cases/scoping/named_scoping_test.rb
index 086977d9a2..9dc26cfd4d 100644
--- a/activerecord/test/cases/scoping/named_scoping_test.rb
+++ b/activerecord/test/cases/scoping/named_scoping_test.rb
@@ -266,6 +266,63 @@ class NamedScopingTest < ActiveRecord::TestCase
     assert_equal 'lifo', topic.author_name
   end
 
+  def test_reserved_scope_names
+    klass = Class.new(ActiveRecord::Base) do
+      self.table_name = "topics"
+
+      scope :approved, -> { where(approved: true) }
+
+      class << self
+        public
+          def pub; end
+
+        private
+          def pri; end
+
+        protected
+          def pro; end
+      end
+    end
+
+    subklass = Class.new(klass)
+
+    conflicts = [
+      :create,        # public class method on AR::Base
+      :relation,      # private class method on AR::Base
+      :new,           # redefined class method on AR::Base
+      :all,           # a default scope
+    ]
+
+    non_conflicts = [
+      :find_by_title, # dynamic finder method
+      :approved,      # existing scope
+      :pub,           # existing public class method
+      :pri,           # existing private class method
+      :pro,           # existing protected class method
+      :open,          # a ::Kernel method
+    ]
+
+    conflicts.each do |name|
+      assert_raises(ArgumentError, "scope `#{name}` should not be allowed") do
+        klass.class_eval { scope name, ->{ where(approved: true) } }
+      end
+
+      assert_raises(ArgumentError, "scope `#{name}` should not be allowed") do
+        subklass.class_eval { scope name, ->{ where(approved: true) } }
+      end
+    end
+
+    non_conflicts.each do |name|
+      assert_nothing_raised do
+        klass.class_eval { scope name, ->{ where(approved: true) } }
+      end
+
+      assert_nothing_raised do
+        subklass.class_eval { scope name, ->{ where(approved: true) } }
+      end
+    end
+  end
+
   # Method delegation for scope names which look like /\A[a-zA-Z_]\w*[!?]?\z/
   # has been done by evaluating a string with a plain def statement. For scope
   # names which contain spaces this approach doesn't work.