Fix a bug where requires inside of autoloads were being added to the...

Fix a bug where requires inside of autoloads were being added to the autoloaded_constants list, causing mayhem. [#5165 state:resolved]

activesupport/lib/active_support/dependencies.rb

@@ -72,10 +72,6 @@ def self.locked(*methods)
         methods.each { |m| class_eval "def #{m}(*) lock { super } end", __FILE__, __LINE__ }
       end
 
-      def get(key)
-        (val = assoc(key)) ? val[1] : []
-      end
-
       locked :concat, :each, :delete_if, :<<
 
       def new_constants_for(frames)
@@ -85,7 +81,18 @@ def new_constants_for(frames)
           next unless mod.is_a?(Module)
 
           new_constants = mod.local_constant_names - prior_constants
-          get(mod_name).concat(new_constants)
+
+          # If we are checking for constants under, say, :Object, nested under something
+          # else that is checking for constants also under :Object, make sure the
+          # parent knows that we have found, and taken care of, the constant.
+          #
+          # In particular, this means that since Kernel.require discards the constants
+          # it finds, parents will be notified that about those constants, and not
+          # consider them "new". As a result, they will not be added to the
+          # autoloaded_constants list.
+          each do |key, value|
+            value.concat(new_constants) if key == mod_name
+          end
 
           new_constants.each do |suffix|
             constants << ([mod_name, suffix] - ["Object"]).join("::")

activesupport/test/autoloading_fixtures/load_path/loaded_constant.rb

+module LoadedConstant
+end
+

activesupport/test/autoloading_fixtures/loads_constant.rb

+module LoadsConstant
+end
+
+RequiresConstant

activesupport/test/autoloading_fixtures/requires_constant.rb

+require "loaded_constant"
+
+module RequiresConstant
+end
+

activesupport/test/dependencies_test.rb

@@ -213,6 +213,48 @@ def test_nested_class_can_access_sibling
     end
   end
 
+  def test_doesnt_break_normal_require
+    path = File.expand_path("../autoloading_fixtures/load_path", __FILE__)
+    original_path = $:.dup
+    original_features = $".dup
+    $:.push(path)
+
+    with_autoloading_fixtures do
+      RequiresConstant
+      assert defined?(RequiresConstant)
+      assert defined?(LoadedConstant)
+      ActiveSupport::Dependencies.clear
+      RequiresConstant
+      assert defined?(RequiresConstant)
+      assert defined?(LoadedConstant)
+    end
+  ensure
+    remove_constants(:RequiresConstant, :LoadedConstant, :LoadsConstant)
+    $".replace(original_features)
+    $:.replace(original_path)
+  end
+
+  def test_doesnt_break_normal_require_nested
+    path = File.expand_path("../autoloading_fixtures/load_path", __FILE__)
+    original_path = $:.dup
+    original_features = $".dup
+    $:.push(path)
+
+    with_autoloading_fixtures do
+      LoadsConstant
+      assert defined?(LoadsConstant)
+      assert defined?(LoadedConstant)
+      ActiveSupport::Dependencies.clear
+      LoadsConstant
+      assert defined?(LoadsConstant)
+      assert defined?(LoadedConstant)
+    end
+  ensure
+    remove_constants(:RequiresConstant, :LoadedConstant, :LoadsConstant)
+    $".replace(original_features)
+    $:.replace(original_path)
+  end
+
   def failing_test_access_thru_and_upwards_fails
     with_autoloading_fixtures do
       assert ! defined?(ModuleFolder)
@@ -797,4 +839,11 @@ def test_unhook
   ensure
     ActiveSupport::Dependencies.hook!
   end
+
+private
+  def remove_constants(*constants)
+    constants.each do |constant|
+      Object.send(:remove_const, constant) if Object.const_defined?(constant)
+    end
+  end
 end