Merge pull request #13107 from chancancode/has_one_counter_cache_should_raise

Raise `ArgumentError` when `has_one` is used with `counter_cache`

5 files changed, 25 insertions, 2 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 5fd0c63c1b..a9713bcf41 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,10 @@
+*   Previously, the `has_one` macro incorrectly accepts the `counter_cache`
+    option due to a bug, although that options was never supported nor
+    functional on `has_one` and `has_one ... through` relationships. It now
+    correctly raises an `ArgumentError` when passed that option.
+
+    *Godfrey Chan*
+
 *   Implement rename_index natively for MySQL >= 5.7.
 
     *Cody Cutrer*
diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb
index aa43c34d86..ac387d377d 100644
--- a/activerecord/lib/active_record/associations/builder/belongs_to.rb
+++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb
@@ -5,7 +5,7 @@ module ActiveRecord::Associations::Builder
     end
 
     def self.valid_options(options)
-      super + [:foreign_type, :polymorphic, :touch]
+      super + [:foreign_type, :polymorphic, :touch, :counter_cache]
     end
 
     def self.valid_dependent_options
diff --git a/activerecord/lib/active_record/associations/builder/singular_association.rb b/activerecord/lib/active_record/associations/builder/singular_association.rb
index 66b03c0301..a6e8300642 100644
--- a/activerecord/lib/active_record/associations/builder/singular_association.rb
+++ b/activerecord/lib/active_record/associations/builder/singular_association.rb
@@ -3,7 +3,7 @@
 module ActiveRecord::Associations::Builder
   class SingularAssociation < Association #:nodoc:
     def self.valid_options(options)
-      super + [:remote, :dependent, :counter_cache, :primary_key, :inverse_of]
+      super + [:remote, :dependent, :primary_key, :inverse_of]
     end
 
     def self.define_accessors(model, reflection)
diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb
index 1f78c73f71..5a41461edf 100644
--- a/activerecord/test/cases/associations/has_one_associations_test.rb
+++ b/activerecord/test/cases/associations/has_one_associations_test.rb
@@ -549,4 +549,12 @@ class HasOneAssociationsTest < ActiveRecord::TestCase
     assert_not_nil author.post
     assert_equal author.post, post
   end
+
+  def test_has_one_relationship_cannot_have_a_counter_cache
+    assert_raise(ArgumentError) do
+      Class.new(ActiveRecord::Base) do
+        has_one :thing, counter_cache: true
+      end
+    end
+  end
 end
diff --git a/activerecord/test/cases/associations/has_one_through_associations_test.rb b/activerecord/test/cases/associations/has_one_through_associations_test.rb
index f2723f2e18..a2725441b3 100644
--- a/activerecord/test/cases/associations/has_one_through_associations_test.rb
+++ b/activerecord/test/cases/associations/has_one_through_associations_test.rb
@@ -315,4 +315,12 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase
   def test_has_one_through_with_custom_select_on_join_model_default_scope
     assert_equal clubs(:boring_club), members(:groucho).selected_club
   end
+
+  def test_has_one_through_relationship_cannot_have_a_counter_cache
+    assert_raise(ArgumentError) do
+      Class.new(ActiveRecord::Base) do
+        has_one :thing, through: :other_thing, counter_cache: true
+      end
+    end
+  end
 end