mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
fixes circularity check in dependencies
The check for circular loading should depend on a stack of files being loaded at the moment, rather than the collection of loaded files. This showed up indirectly in #16468, where a misspelled helper would incorrectly result in a circularity error message. References #16468
This commit is contained in:
parent
c4767e13d4
commit
ae07806858
4 changed files with 37 additions and 1 deletions
|
@ -30,6 +30,10 @@ module ActiveSupport #:nodoc:
|
||||||
mattr_accessor :loaded
|
mattr_accessor :loaded
|
||||||
self.loaded = Set.new
|
self.loaded = Set.new
|
||||||
|
|
||||||
|
# Stack of files being loaded.
|
||||||
|
mattr_accessor :loading
|
||||||
|
self.loading = []
|
||||||
|
|
||||||
# Should we load files or require them?
|
# Should we load files or require them?
|
||||||
mattr_accessor :mechanism
|
mattr_accessor :mechanism
|
||||||
self.mechanism = ENV['NO_RELOAD'] ? :require : :load
|
self.mechanism = ENV['NO_RELOAD'] ? :require : :load
|
||||||
|
@ -317,6 +321,7 @@ module ActiveSupport #:nodoc:
|
||||||
def clear
|
def clear
|
||||||
log_call
|
log_call
|
||||||
loaded.clear
|
loaded.clear
|
||||||
|
loading.clear
|
||||||
remove_unloadable_constants!
|
remove_unloadable_constants!
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -329,6 +334,7 @@ module ActiveSupport #:nodoc:
|
||||||
# Record that we've seen this file *before* loading it to avoid an
|
# Record that we've seen this file *before* loading it to avoid an
|
||||||
# infinite loop with mutual dependencies.
|
# infinite loop with mutual dependencies.
|
||||||
loaded << expanded
|
loaded << expanded
|
||||||
|
loading << expanded
|
||||||
|
|
||||||
begin
|
begin
|
||||||
if load?
|
if load?
|
||||||
|
@ -351,6 +357,8 @@ module ActiveSupport #:nodoc:
|
||||||
rescue Exception
|
rescue Exception
|
||||||
loaded.delete expanded
|
loaded.delete expanded
|
||||||
raise
|
raise
|
||||||
|
ensure
|
||||||
|
loading.pop
|
||||||
end
|
end
|
||||||
|
|
||||||
# Record history *after* loading so first load gets warnings.
|
# Record history *after* loading so first load gets warnings.
|
||||||
|
@ -475,7 +483,7 @@ module ActiveSupport #:nodoc:
|
||||||
expanded = File.expand_path(file_path)
|
expanded = File.expand_path(file_path)
|
||||||
expanded.sub!(/\.rb\z/, '')
|
expanded.sub!(/\.rb\z/, '')
|
||||||
|
|
||||||
if loaded.include?(expanded)
|
if loading.include?(expanded)
|
||||||
raise "Circular dependency detected while autoloading constant #{qualified_name}"
|
raise "Circular dependency detected while autoloading constant #{qualified_name}"
|
||||||
else
|
else
|
||||||
require_or_load(expanded, qualified_name)
|
require_or_load(expanded, qualified_name)
|
||||||
|
|
2
activesupport/test/autoloading_fixtures/typo.rb
Normal file
2
activesupport/test/autoloading_fixtures/typo.rb
Normal file
|
@ -0,0 +1,2 @@
|
||||||
|
TypO = 1
|
||||||
|
|
|
@ -157,6 +157,31 @@ class DependenciesTest < ActiveSupport::TestCase
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def test_ensures_the_expected_constant_is_defined
|
||||||
|
with_autoloading_fixtures do
|
||||||
|
e = assert_raise(LoadError) { Typo }
|
||||||
|
assert_match %r{Unable to autoload constant Typo, expected .*activesupport/test/autoloading_fixtures/typo.rb to define it}, e.message
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_require_dependency_does_not_assume_any_particular_constant_is_defined
|
||||||
|
with_autoloading_fixtures do
|
||||||
|
require_dependency 'typo'
|
||||||
|
assert_equal 1, TypO
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
# Regression, see https://github.com/rails/rails/issues/16468.
|
||||||
|
def test_require_dependency_interaction_with_autoloading
|
||||||
|
with_autoloading_fixtures do
|
||||||
|
require_dependency 'typo'
|
||||||
|
assert_equal 1, TypO
|
||||||
|
|
||||||
|
e = assert_raise(LoadError) { Typo }
|
||||||
|
assert_match %r{Unable to autoload constant Typo, expected .*activesupport/test/autoloading_fixtures/typo.rb to define it}, e.message
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
def test_module_loading
|
def test_module_loading
|
||||||
with_autoloading_fixtures do
|
with_autoloading_fixtures do
|
||||||
assert_kind_of Module, A
|
assert_kind_of Module, A
|
||||||
|
|
|
@ -13,6 +13,7 @@ module DependenciesTestHelpers
|
||||||
ActiveSupport::Dependencies.autoload_paths = prior_autoload_paths
|
ActiveSupport::Dependencies.autoload_paths = prior_autoload_paths
|
||||||
ActiveSupport::Dependencies.mechanism = old_mechanism
|
ActiveSupport::Dependencies.mechanism = old_mechanism
|
||||||
ActiveSupport::Dependencies.explicitly_unloadable_constants = []
|
ActiveSupport::Dependencies.explicitly_unloadable_constants = []
|
||||||
|
ActiveSupport::Dependencies.clear
|
||||||
end
|
end
|
||||||
|
|
||||||
def with_autoloading_fixtures(&block)
|
def with_autoloading_fixtures(&block)
|
||||||
|
|
Loading…
Reference in a new issue