Don't leak Object constants in core_ext/module/qualified_const

3 files changed, 107 insertions, 74 deletions

diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 8e0e5bb2ef..07f7fae5d5 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,3 +1,8 @@
+*   Deprecated `Module#qualified_const_` in favour of the builtin Module#const_
+    methods.
+
+    *Genadi Samokovarov*
+
 *   Deprecate passing string to define callback.
 
     *Yuichiro Kaneko*
@@ -6,9 +11,9 @@
     `ActiveSupport::Cache::MemCachedStore#escape_key`, and 
     `ActiveSupport::Cache::FileStore#key_file_path` 
     are deprecated and replaced with `normalize_key` that now calls `super`.
-    
+
     `ActiveSupport::Cache::LocaleCache#set_cache_value` is deprecated and replaced with `write_cache_value`.
-    
+
     *Michael Grosser*
 
 *   Implements an evented file watcher to asynchronously detect changes in the
diff --git a/activesupport/lib/active_support/core_ext/module/qualified_const.rb b/activesupport/lib/active_support/core_ext/module/qualified_const.rb
index 65525013db..3ea39d4267 100644
--- a/activesupport/lib/active_support/core_ext/module/qualified_const.rb
+++ b/activesupport/lib/active_support/core_ext/module/qualified_const.rb
@@ -3,13 +3,16 @@ require 'active_support/core_ext/string/inflections'
 #--
 # Allows code reuse in the methods below without polluting Module.
 #++
-module QualifiedConstUtils
-  def self.raise_if_absolute(path)
-    raise NameError.new("wrong constant name #$&") if path =~ /\A::[^:]+/
-  end
 
-  def self.names(path)
-    path.split('::')
+module ActiveSupport
+  module QualifiedConstUtils
+    def self.raise_if_absolute(path)
+      raise NameError.new("wrong constant name #$&") if path =~ /\A::[^:]+/
+    end
+
+    def self.names(path)
+      path.split('::')
+    end
   end
 end
 
@@ -24,9 +27,14 @@ end
 #++
 class Module
   def qualified_const_defined?(path, search_parents=true)
-    QualifiedConstUtils.raise_if_absolute(path)
+    ActiveSupport::Deprecation.warn(<<-MESSAGE.squish)
+      Module#qualified_const_defined? is deprecated in favour of the builtin
+      Module#const_defined? and will be removed in Rails 5.1.
+    MESSAGE
 
-    QualifiedConstUtils.names(path).inject(self) do |mod, name|
+    ActiveSupport::QualifiedConstUtils.raise_if_absolute(path)
+
+    ActiveSupport::QualifiedConstUtils.names(path).inject(self) do |mod, name|
       return unless mod.const_defined?(name, search_parents)
       mod.const_get(name)
     end
@@ -34,19 +42,29 @@ class Module
   end
 
   def qualified_const_get(path)
-    QualifiedConstUtils.raise_if_absolute(path)
+    ActiveSupport::Deprecation.warn(<<-MESSAGE.squish)
+      Module#qualified_const_get is deprecated in favour of the builtin
+      Module#const_get and will be removed in Rails 5.1.
+    MESSAGE
+
+    ActiveSupport::QualifiedConstUtils.raise_if_absolute(path)
 
-    QualifiedConstUtils.names(path).inject(self) do |mod, name|
+    ActiveSupport::QualifiedConstUtils.names(path).inject(self) do |mod, name|
       mod.const_get(name)
     end
   end
 
   def qualified_const_set(path, value)
-    QualifiedConstUtils.raise_if_absolute(path)
+    ActiveSupport::Deprecation.warn(<<-MESSAGE.squish)
+      Module#qualified_const_set is deprecated in favour of the builtin
+      Module#const_set and will be removed in Rails 5.1.
+    MESSAGE
+
+    ActiveSupport::QualifiedConstUtils.raise_if_absolute(path)
 
     const_name = path.demodulize
     mod_name = path.deconstantize
-    mod = mod_name.empty? ? self : qualified_const_get(mod_name)
+    mod = mod_name.empty? ? self : const_get(mod_name)
     mod.const_set(const_name, value)
   end
 end
diff --git a/activesupport/test/core_ext/module/qualified_const_test.rb b/activesupport/test/core_ext/module/qualified_const_test.rb
index 37c9228a64..a3146cabe1 100644
--- a/activesupport/test/core_ext/module/qualified_const_test.rb
+++ b/activesupport/test/core_ext/module/qualified_const_test.rb
@@ -19,84 +19,94 @@ end
 
 class QualifiedConstTest < ActiveSupport::TestCase
   test "Object.qualified_const_defined?" do
-    assert Object.qualified_const_defined?("QualifiedConstTestMod")
-    assert !Object.qualified_const_defined?("NonExistingQualifiedConstTestMod")
-
-    assert Object.qualified_const_defined?("QualifiedConstTestMod::X")
-    assert !Object.qualified_const_defined?("QualifiedConstTestMod::Y")
-
-    assert Object.qualified_const_defined?("QualifiedConstTestMod::M::X")
-    assert !Object.qualified_const_defined?("QualifiedConstTestMod::M::Y")
-
-    if Module.method(:const_defined?).arity == 1
-      assert !Object.qualified_const_defined?("QualifiedConstTestMod::N::X")
-    else
-      assert Object.qualified_const_defined?("QualifiedConstTestMod::N::X")
-      assert !Object.qualified_const_defined?("QualifiedConstTestMod::N::X", false)
-      assert Object.qualified_const_defined?("QualifiedConstTestMod::N::X", true)
+    assert_deprecated do
+      assert Object.qualified_const_defined?("QualifiedConstTestMod")
+      assert !Object.qualified_const_defined?("NonExistingQualifiedConstTestMod")
+
+      assert Object.qualified_const_defined?("QualifiedConstTestMod::X")
+      assert !Object.qualified_const_defined?("QualifiedConstTestMod::Y")
+
+      assert Object.qualified_const_defined?("QualifiedConstTestMod::M::X")
+      assert !Object.qualified_const_defined?("QualifiedConstTestMod::M::Y")
+
+      if Module.method(:const_defined?).arity == 1
+        assert !Object.qualified_const_defined?("QualifiedConstTestMod::N::X")
+      else
+        assert Object.qualified_const_defined?("QualifiedConstTestMod::N::X")
+        assert !Object.qualified_const_defined?("QualifiedConstTestMod::N::X", false)
+        assert Object.qualified_const_defined?("QualifiedConstTestMod::N::X", true)
+      end
     end
   end
 
   test "mod.qualified_const_defined?" do
-    assert QualifiedConstTestMod.qualified_const_defined?("M")
-    assert !QualifiedConstTestMod.qualified_const_defined?("NonExistingM")
-
-    assert QualifiedConstTestMod.qualified_const_defined?("M::X")
-    assert !QualifiedConstTestMod.qualified_const_defined?("M::Y")
-
-    assert QualifiedConstTestMod.qualified_const_defined?("M::C::X")
-    assert !QualifiedConstTestMod.qualified_const_defined?("M::C::Y")
-
-    if Module.method(:const_defined?).arity == 1
-      assert !QualifiedConstTestMod.qualified_const_defined?("QualifiedConstTestMod::N::X")
-    else
-      assert QualifiedConstTestMod.qualified_const_defined?("N::X")
-      assert !QualifiedConstTestMod.qualified_const_defined?("N::X", false)
-      assert QualifiedConstTestMod.qualified_const_defined?("N::X", true)
+    assert_deprecated do
+      assert QualifiedConstTestMod.qualified_const_defined?("M")
+      assert !QualifiedConstTestMod.qualified_const_defined?("NonExistingM")
+
+      assert QualifiedConstTestMod.qualified_const_defined?("M::X")
+      assert !QualifiedConstTestMod.qualified_const_defined?("M::Y")
+
+      assert QualifiedConstTestMod.qualified_const_defined?("M::C::X")
+      assert !QualifiedConstTestMod.qualified_const_defined?("M::C::Y")
+
+      if Module.method(:const_defined?).arity == 1
+        assert !QualifiedConstTestMod.qualified_const_defined?("QualifiedConstTestMod::N::X")
+      else
+        assert QualifiedConstTestMod.qualified_const_defined?("N::X")
+        assert !QualifiedConstTestMod.qualified_const_defined?("N::X", false)
+        assert QualifiedConstTestMod.qualified_const_defined?("N::X", true)
+      end
     end
   end
 
   test "qualified_const_get" do
-    assert_equal false, Object.qualified_const_get("QualifiedConstTestMod::X")
-    assert_equal false, QualifiedConstTestMod.qualified_const_get("X")
-    assert_equal 1, QualifiedConstTestMod.qualified_const_get("M::X")
-    assert_equal 1, QualifiedConstTestMod.qualified_const_get("N::X")
-    assert_equal 2, QualifiedConstTestMod.qualified_const_get("M::C::X")
-
-    assert_raise(NameError) { QualifiedConstTestMod.qualified_const_get("M::C::Y")}
+    assert_deprecated do
+      assert_equal false, Object.qualified_const_get("QualifiedConstTestMod::X")
+      assert_equal false, QualifiedConstTestMod.qualified_const_get("X")
+      assert_equal 1, QualifiedConstTestMod.qualified_const_get("M::X")
+      assert_equal 1, QualifiedConstTestMod.qualified_const_get("N::X")
+      assert_equal 2, QualifiedConstTestMod.qualified_const_get("M::C::X")
+
+      assert_raise(NameError) { QualifiedConstTestMod.qualified_const_get("M::C::Y")}
+    end
   end
 
   test "qualified_const_set" do
-    begin
-      m = Module.new
-      assert_equal m, Object.qualified_const_set("QualifiedConstTestMod2", m)
-      assert_equal m, ::QualifiedConstTestMod2
-
-      # We are going to assign to existing constants on purpose, so silence warnings.
-      silence_warnings do
-        assert_equal true, QualifiedConstTestMod.qualified_const_set("QualifiedConstTestMod::X", true)
-        assert_equal true, QualifiedConstTestMod::X
-
-        assert_equal 10, QualifiedConstTestMod::M.qualified_const_set("X", 10)
-        assert_equal 10, QualifiedConstTestMod::M::X
-      end
-    ensure
-      silence_warnings do
-        QualifiedConstTestMod.qualified_const_set('QualifiedConstTestMod::X', false)
-        QualifiedConstTestMod::M.qualified_const_set('X', 1)
+    assert_deprecated do
+      begin
+        m = Module.new
+        assert_equal m, Object.qualified_const_set("QualifiedConstTestMod2", m)
+        assert_equal m, ::QualifiedConstTestMod2
+
+        # We are going to assign to existing constants on purpose, so silence warnings.
+        silence_warnings do
+          assert_equal true, QualifiedConstTestMod.qualified_const_set("QualifiedConstTestMod::X", true)
+          assert_equal true, QualifiedConstTestMod::X
+
+          assert_equal 10, QualifiedConstTestMod::M.qualified_const_set("X", 10)
+          assert_equal 10, QualifiedConstTestMod::M::X
+        end
+      ensure
+        silence_warnings do
+          QualifiedConstTestMod.qualified_const_set('QualifiedConstTestMod::X', false)
+          QualifiedConstTestMod::M.qualified_const_set('X', 1)
+        end
       end
     end
   end
 
   test "reject absolute paths" do
-    assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_defined?("::X")}
-    assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_defined?("::X::Y")}
+    assert_deprecated do
+      assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_defined?("::X")}
+      assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_defined?("::X::Y")}
 
-    assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_get("::X")}
-    assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_get("::X::Y")}
+      assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_get("::X")}
+      assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_get("::X::Y")}
 
-    assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_set("::X", nil)}
-    assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_set("::X::Y", nil)}
+      assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_set("::X", nil)}
+      assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_set("::X::Y", nil)}
+    end
   end
 
   private