let remove_constant still delete Kernel#autoload constants [rounds #8213]

The method #remove_const does not load the file, so we
can still remove the constant.

3 files changed, 25 insertions, 13 deletions

diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 7daa0d5bca..cde98c6667 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,6 +1,6 @@
 ## Rails 4.0.0 (unreleased) ##
 
-*   Dependencies no longer trigger Kernel#autoload in remove_const [fixes #8213]. *Xavier Noria*
+*   Dependencies no longer trigger Kernel#autoload in remove_constant [fixes #8213]. *Xavier Noria*
 
 *   Simplify mocha integration and remove monkey-patches, bumping mocha to 0.13.0. *James Mead*
 
diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb
index 00b1b3ec98..c75fb46263 100644
--- a/activesupport/lib/active_support/dependencies.rb
+++ b/activesupport/lib/active_support/dependencies.rb
@@ -649,6 +649,8 @@ module ActiveSupport #:nodoc:
       parent_name = constants.empty? ? 'Object' : constants.join('::')
 
       if parent = safe_constantize(parent_name)
+        log "removing constant #{const}"
+
         # In an autoloaded user.rb like this
         #
         #   autoload :Foo, 'foo'
@@ -656,20 +658,30 @@ module ActiveSupport #:nodoc:
         #   class User < ActiveRecord::Base
         #   end
         #
-        # we correctly register "Foo" as being autoloaded. But if the app
-        # does not use the "Foo" constant we need to be careful not to
-        # trigger loading "foo". If the autoload has not been triggered
-        # we already know there is nothing to remove so just return.
-        return if parent.autoload?(to_remove)
+        # we correctly register "Foo" as being autoloaded. But if the app does
+        # not use the "Foo" constant we need to be careful not to trigger
+        # loading "foo.rb" ourselves. While #const_defined? and #const_get? do
+        # require the file, #autoload? and #remove_const don't.
+        #
+        # We are going to remove the constant nonetheless ---which exists as
+        # far as Ruby is concerned--- because if the user removes the macro
+        # call from a class or module that were not autoloaded, as in the
+        # example above with Object, accessing to that constant must err.
+        unless parent.autoload?(to_remove)
+          begin
+            constantized = parent.const_get(to_remove, false)
+          rescue NameError
+            log "the constant #{const} is not reachable anymore, skipping"
+            return
+          else
+            constantized.before_remove_const if constantized.respond_to?(:before_remove_const)
+          end
+        end
 
         begin
-          log "removing constant #{const}"
-          constantized = parent.const_get(to_remove, false)
+          parent.instance_eval { remove_const to_remove }
         rescue NameError
           log "the constant #{const} is not reachable anymore, skipping"
-        else
-          constantized.before_remove_const if constantized.respond_to?(:before_remove_const)
-          parent.instance_eval { remove_const to_remove }
         end
       end
     end
diff --git a/activesupport/test/dependencies_test.rb b/activesupport/test/dependencies_test.rb
index 6e5a6f71e2..67bd6669c5 100644
--- a/activesupport/test/dependencies_test.rb
+++ b/activesupport/test/dependencies_test.rb
@@ -933,9 +933,9 @@ class DependenciesTest < ActiveSupport::TestCase
     Object.class_eval do
       autoload constant, File.expand_path('../autoloading_fixtures/should_not_be_required', __FILE__)
     end
-    ActiveSupport::Dependencies.remove_constant(constant)
 
-    assert Object.autoload?(constant), "Kernel#autoload of #{constant} has been triggered by remove_const"
+    assert_nil ActiveSupport::Dependencies.remove_constant(constant), "Kernel#autoload has been triggered by remove_constant"
+    assert !defined?(ShouldNotBeAutoloaded)
   end
 
   def test_load_once_constants_should_not_be_unloaded