From a8c3ea90f1490da4404aa1cea6fc6209f6b9b99b Mon Sep 17 00:00:00 2001 From: Xavier Noria Date: Thu, 15 Nov 2012 11:06:35 +0100 Subject: 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. --- activesupport/CHANGELOG.md | 2 +- activesupport/lib/active_support/dependencies.rb | 32 ++++++++++++++++-------- activesupport/test/dependencies_test.rb | 4 +-- 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 -- cgit v1.2.3