diff options
| -rw-r--r-- | activesupport/lib/active_support/dependencies.rb | 76 | ||||
| -rw-r--r-- | activesupport/test/dependencies_test.rb | 10 |
2 files changed, 54 insertions, 32 deletions
diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index c75fb46263..b816ecae5a 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -644,46 +644,58 @@ module ActiveSupport #:nodoc: normalized = const.to_s.sub(/\A::/, '') normalized.sub!(/\A(Object::)+/, '') - constants = normalized.split('::') - to_remove = constants.pop - parent_name = constants.empty? ? 'Object' : constants.join('::') + constants = normalized.split('::') + to_remove = constants.pop - if parent = safe_constantize(parent_name) - log "removing constant #{const}" - - # In an autoloaded user.rb like this - # - # autoload :Foo, 'foo' - # - # 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.rb" ourselves. While #const_defined? and #const_get? do - # require the file, #autoload? and #remove_const don't. + if constants.empty? + parent = Object + else + # This method is robust to non-reachable constants. # - # 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 + # Non-reachable constants may be passed if some of the parents were + # autoloaded and already removed. It is easier to do a sanity check + # here than require the caller to be clever. We check the parent + # rather than the very const argument because we do not want to + # trigger Kernel#autoloads, see the comment below. + parent_name = constants.join('::') + return unless qualified_const_defined?(parent_name) + parent = constantize(parent_name) + end + log "removing constant #{const}" + + # In an autoloaded user.rb like this + # + # autoload :Foo, 'foo' + # + # 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.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 - parent.instance_eval { remove_const to_remove } + 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 + parent.instance_eval { remove_const to_remove } + rescue NameError + log "the constant #{const} is not reachable anymore, skipping" + end end protected diff --git a/activesupport/test/dependencies_test.rb b/activesupport/test/dependencies_test.rb index 67bd6669c5..7fea72dab5 100644 --- a/activesupport/test/dependencies_test.rb +++ b/activesupport/test/dependencies_test.rb @@ -938,6 +938,16 @@ class DependenciesTest < ActiveSupport::TestCase assert !defined?(ShouldNotBeAutoloaded) end + def test_remove_constant_does_not_autoload_already_removed_parents_as_a_side_effect + with_autoloading_fixtures do + ::A + ::A::B + ActiveSupport::Dependencies.remove_constant('A') + ActiveSupport::Dependencies.remove_constant('A::B') + assert !defined?(A) + end + end + def test_load_once_constants_should_not_be_unloaded with_autoloading_fixtures do ActiveSupport::Dependencies.autoload_once_paths = ActiveSupport::Dependencies.autoload_paths |