Skip to content

Commit

Permalink
Restore support for namespaces that are not hashable
Browse files Browse the repository at this point in the history
2.5.0 introduced a hash table whose keys were pairs (parent, cname), in order to
be able to say if a given pair had an autoload set by us.

That broke a project that has a namespace with the `hash` method overridden with
a different arity. Hence, not hashable.

This patch restores the structure used in 2.4, and implements a different way to
answer that question. Indeed, this alternative is simpler.

Fixes #188.
  • Loading branch information
fxn committed Oct 20, 2021
1 parent e9fc5de commit c0bdbfb
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 161 deletions.
1 change: 0 additions & 1 deletion lib/zeitwerk.rb
Expand Up @@ -3,7 +3,6 @@
module Zeitwerk
require_relative "zeitwerk/real_mod_name"
require_relative "zeitwerk/loader"
require_relative "zeitwerk/autoloads"
require_relative "zeitwerk/registry"
require_relative "zeitwerk/explicit_namespace"
require_relative "zeitwerk/inflector"
Expand Down
71 changes: 0 additions & 71 deletions lib/zeitwerk/autoloads.rb

This file was deleted.

40 changes: 22 additions & 18 deletions lib/zeitwerk/loader.rb
Expand Up @@ -14,22 +14,16 @@ class Loader
include Helpers
include Config

# Keeps track of autoloads defined by the loader which have not been
# executed so far.
# Maps absolute paths for which an autoload has been set ---and not
# executed--- to their corresponding parent class or module and constant
# name.
#
# This metadata helps us implement a few things:
#
# 1. When autoloads are triggered, ensure they define the expected constant
# and invoke user callbacks. If reloading is enabled, remember cref and
# abspath for later unloading logic.
#
# 2. When unloading, remove autoloads that have not been executed.
#
# 3. Eager load with a recursive const_get, rather than a recursive require,
# for consistency with lazy loading.
# "/Users/fxn/blog/app/models/user.rb" => [Object, :User],
# "/Users/fxn/blog/app/models/hotel/pricing.rb" => [Hotel, :Pricing]
# ...
#
# @private
# @sig Zeitwerk::Autoloads
# @sig Hash[String, [Module, Symbol]]
attr_reader :autoloads

# We keep track of autoloaded directories to remove them from the registry
Expand Down Expand Up @@ -87,7 +81,7 @@ class Loader
def initialize
super

@autoloads = Autoloads.new
@autoloads = {}
@autoloaded_dirs = []
@to_unload = {}
@lazy_subdirs = Hash.new { |h, cpath| h[cpath] = [] }
Expand Down Expand Up @@ -138,7 +132,7 @@ def unload
# is enough.
unloaded_files = Set.new

autoloads.each do |(parent, cname), abspath|
autoloads.each do |abspath, (parent, cname)|
if parent.autoload?(cname)
unload_autoload(parent, cname)
else
Expand Down Expand Up @@ -234,7 +228,7 @@ def eager_load(force: false)
next if honour_exclusions && excluded_from_eager_load?(abspath)

if ruby?(abspath)
if cref = autoloads.cref_for(abspath)
if cref = autoloads[abspath]
cget(*cref)
end
elsif dir?(abspath) && !root_dirs.key?(abspath)
Expand Down Expand Up @@ -376,7 +370,7 @@ def set_autoloads_in_dir(dir, parent)

# @sig (Module, Symbol, String) -> void
def autoload_subdir(parent, cname, subdir)
if autoload_path = autoloads.abspath_for(parent, cname)
if autoload_path = autoload_path_set_by_me_for?(parent, cname)
cpath = cpath(parent, cname)
register_explicit_namespace(cpath) if ruby?(autoload_path)
# We do not need to issue another autoload, the existing one is enough
Expand Down Expand Up @@ -432,7 +426,7 @@ def promote_namespace_from_implicit_to_explicit(dir:, file:, parent:, cname:)

# @sig (Module, Symbol, String) -> void
def set_autoload(parent, cname, abspath)
autoloads.define(parent, cname, abspath)
parent.autoload(cname, abspath)

if logger
if ruby?(abspath)
Expand All @@ -442,6 +436,7 @@ def set_autoload(parent, cname, abspath)
end
end

autoloads[abspath] = [parent, cname]
Registry.register_autoload(self, abspath)

# See why in the documentation of Zeitwerk::Registry.inceptions.
Expand All @@ -450,6 +445,15 @@ def set_autoload(parent, cname, abspath)
end
end

# @sig (Module, Symbol) -> String?
def autoload_path_set_by_me_for?(parent, cname)
if autoload_path = strict_autoload_path(parent, cname)
autoload_path if autoloads.key?(autoload_path)
else
Registry.inception?(cpath(parent, cname))
end
end

# @sig (String) -> void
def register_explicit_namespace(cpath)
ExplicitNamespace.register(cpath, self)
Expand Down
71 changes: 0 additions & 71 deletions test/lib/zeitwerk/test_autoloads.rb

This file was deleted.

17 changes: 17 additions & 0 deletions test/lib/zeitwerk/test_explicit_namespace.rb
Expand Up @@ -203,4 +203,21 @@ def x
assert tracer.enabled?
end
end

test "non-hashable explicit namespaces are supported" do
files = [
["m.rb", <<~EOS],
module M
# This method is overridden with a different arity. Therefore, M is
# not hashable. See https://github.com/fxn/zeitwerk/issues/188.
def self.hash(_)
end
end
EOS
["m/x.rb", "M::X = true"]
]
with_setup(files) do
assert M::X
end
end
end

0 comments on commit c0bdbfb

Please sign in to comment.