From db92ea32e0c2a04c47d9658e17ee9cb26e70e3ab Mon Sep 17 00:00:00 2001
From: Jean Boussier <jean.boussier@gmail.com>
Date: Tue, 31 Oct 2023 12:59:27 +0100
Subject: [PATCH] Simplify attr_internal_define

The `@` prefix is always stripped, so might as well not require it.

For backward compatibility reasons we still handle the prefix for now,
but we eagerly strip it and emit a deprecation.
---
 .../core_ext/module/attr_internal.rb          | 23 ++++++++++++++-----
 .../core_ext/module/attr_internal_test.rb     | 18 +++++++++++----
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/activesupport/lib/active_support/core_ext/module/attr_internal.rb b/activesupport/lib/active_support/core_ext/module/attr_internal.rb
index 3bd66ff3bcee5..905e79e987fba 100644
--- a/activesupport/lib/active_support/core_ext/module/attr_internal.rb
+++ b/activesupport/lib/active_support/core_ext/module/attr_internal.rb
@@ -19,16 +19,27 @@ def attr_internal_accessor(*attrs)
   end
   alias_method :attr_internal, :attr_internal_accessor
 
-  class << self; attr_accessor :attr_internal_naming_format end
-  self.attr_internal_naming_format = "@_%s"
+  class << self
+    attr_reader :attr_internal_naming_format
 
-  private
-    def attr_internal_ivar_name(attr)
-      Module.attr_internal_naming_format % attr
+    def attr_internal_naming_format=(format)
+      if format.start_with?("@")
+        ActiveSupport.deprecator.warn <<~MESSAGE
+          Setting `attr_internal_naming_format` with a `@` prefix is deprecated and will be removed in Rails 7.2.
+
+          You can simply replace #{format.inspect} by #{format.delete_prefix("@").inspect}.
+        MESSAGE
+
+        format = format.delete_prefix("@")
+      end
+      @attr_internal_naming_format = format
     end
+  end
+  self.attr_internal_naming_format = "_%s"
 
+  private
     def attr_internal_define(attr_name, type)
-      internal_name = attr_internal_ivar_name(attr_name).delete_prefix("@")
+      internal_name = Module.attr_internal_naming_format % attr_name
       # use native attr_* methods as they are faster on some Ruby implementations
       public_send("attr_#{type}", internal_name)
       attr_name, internal_name = "#{attr_name}=", "#{internal_name}=" if type == :writer
diff --git a/activesupport/test/core_ext/module/attr_internal_test.rb b/activesupport/test/core_ext/module/attr_internal_test.rb
index 1893eed2b44d1..ad070578d20f1 100644
--- a/activesupport/test/core_ext/module/attr_internal_test.rb
+++ b/activesupport/test/core_ext/module/attr_internal_test.rb
@@ -7,6 +7,11 @@ class AttrInternalTest < ActiveSupport::TestCase
   def setup
     @target = Class.new
     @instance = @target.new
+    @naming_format_was = Module.attr_internal_naming_format
+  end
+
+  def teardown
+    Module.attr_internal_naming_format = @naming_format_was
   end
 
   def test_reader
@@ -39,9 +44,16 @@ def test_accessor
     assert_nothing_raised { assert_equal 1, @instance.foo }
   end
 
+  def test_naming_format_deprecation
+    assert_equal "_%s", Module.attr_internal_naming_format
+    assert_deprecated(ActiveSupport.deprecator) do
+      Module.attr_internal_naming_format = "@___%s"
+    end
+    assert_equal "___%s", Module.attr_internal_naming_format
+  end
+
   def test_naming_format
-    assert_equal "@_%s", Module.attr_internal_naming_format
-    assert_nothing_raised { Module.attr_internal_naming_format = "@abc%sdef" }
+    assert_nothing_raised { Module.attr_internal_naming_format = "abc%sdef" }
     @target.attr_internal :foo
 
     assert_not @instance.instance_variable_defined?("@_foo")
@@ -49,7 +61,5 @@ def test_naming_format
     assert_nothing_raised { @instance.foo = 1 }
     assert_not @instance.instance_variable_defined?("@_foo")
     assert @instance.instance_variable_defined?("@abcfoodef")
-  ensure
-    Module.attr_internal_naming_format = "@_%s"
   end
 end