Skip to content

Commit

Permalink
Reword Zeitwerk::NameError message again
Browse files Browse the repository at this point in the history
I am sleeping on this one and trying different options.

The previous revision lacked symmetry for my taste, because if you say
"the top-level constant X", when there is a namespace "M::N", why is
that "M" not qualified as top-level?

The answer is that an error message is not a constant lookup, it is by
convention relative to the top-level namespace. But then, the messages
for simple and compound constant paths lack symmetry.

This alternative is more coherent, and also matches the way you think
about project layouts. You define constants in namespaces represented by
directories.

Let's try this one now.
  • Loading branch information
fxn committed Apr 1, 2024
1 parent 1671229 commit 3d43c2f
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 8 deletions.
4 changes: 2 additions & 2 deletions lib/zeitwerk/loader/callbacks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ module Zeitwerk::Loader::Callbacks
to_unload[cpath] = [file, cref] if reloading_enabled?
run_on_load_callbacks(cpath, cget(*cref), file) unless on_load_callbacks.empty?
else
the = cref[0] == Object ? "the top-level" : "the"
msg = "expected file #{file} to define #{the} constant #{cpath}"
namespace = cref[0] == Object ? "top-level namespace" : "namespace #{real_mod_name(cref[0])}"
msg = "expected #{file} to define #{cref[1]} in the #{namespace}"
log(msg) if logger

# Ruby still keeps the autoload defined, but we remove it because the
Expand Down
10 changes: 5 additions & 5 deletions test/lib/zeitwerk/test_exceptions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def assert_error_message(message, error)
with_setup(files) do
typo_rb = File.expand_path("typo.rb")
error = assert_raises(Zeitwerk::NameError) { Typo }
assert_error_message "expected file #{typo_rb} to define the top-level constant Typo", error
assert_error_message "expected #{typo_rb} to define Typo in the top-level namespace", error
assert_equal :Typo, error.name
end
end
Expand All @@ -29,7 +29,7 @@ def assert_error_message(message, error)
with_setup(files) do
typo_rb = File.expand_path("m/typo.rb")
error = assert_raises(Zeitwerk::NameError) { M::Typo }
assert_error_message "expected file #{typo_rb} to define the constant M::Typo", error
assert_error_message "expected #{typo_rb} to define Typo in the namespace M", error
assert_equal :Typo, error.name
end
end
Expand All @@ -39,7 +39,7 @@ def assert_error_message(message, error)
with_setup(files) do
x_rb = File.expand_path("x.rb")
error = assert_raises(Zeitwerk::NameError) { loader.eager_load }
assert_error_message "expected file #{x_rb} to define the top-level constant X", error
assert_error_message "expected #{x_rb} to define X in the top-level namespace", error
assert_equal :X, error.name
end
end
Expand All @@ -49,7 +49,7 @@ def assert_error_message(message, error)
with_setup(files) do
x_rb = File.expand_path("m/x.rb")
error = assert_raises(Zeitwerk::NameError) { loader.eager_load }
assert_error_message "expected file #{x_rb} to define the constant M::X", error
assert_error_message "expected #{x_rb} to define X in the namespace M", error
assert_equal :X, error.name
end
end
Expand All @@ -64,7 +64,7 @@ def assert_error_message(message, error)
with_setup(files) do
cli_x_rb = File.expand_path("cli/x.rb")
error = assert_raises(Zeitwerk::NameError) { loader.eager_load }
assert_error_message "expected file #{cli_x_rb} to define the constant Cli::X", error
assert_error_message "expected #{cli_x_rb} to define X in the namespace Cli", error
assert_equal :X, error.name
end
end
Expand Down
2 changes: 1 addition & 1 deletion test/lib/zeitwerk/test_logging.rb
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ def logger.debug(message)
test "logs failed autoloads, provided the require call succeeded" do
files = [["x.rb", ""]]
with_files(files) do
assert_logged(/expected file #{File.expand_path("x.rb")} to define the top-level constant X/) do
assert_logged(/expected #{File.expand_path("x.rb")} to define X in the top-level namespace/) do
loader.push_dir(".")
loader.setup
assert_raises(Zeitwerk::NameError) { X }
Expand Down

0 comments on commit 3d43c2f

Please sign in to comment.