Skip to content

Commit

Permalink
Polish Zeitwerk::NameError message
Browse files Browse the repository at this point in the history
The current error for missing top-level constants is:

  expected file #{x_rb} to define constant X, but didn't

It could be argued that "constant X" is ambiguous,
because

    M::X = 1

defines a constant X.

By saying "the top-level constant X" we remove the
ambiguity, and the error message still reads well
for my taste.
  • Loading branch information
fxn committed Mar 26, 2024
1 parent 0b6477f commit 1671229
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 7 deletions.
3 changes: 2 additions & 1 deletion lib/zeitwerk/loader/callbacks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +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
msg = "expected file #{file} to define constant #{cpath}, but didn't"
the = cref[0] == Object ? "the top-level" : "the"
msg = "expected file #{file} to define #{the} constant #{cpath}"
log(msg) if logger

# Ruby still keeps the autoload defined, but we remove it because the
Expand Down
30 changes: 25 additions & 5 deletions test/lib/zeitwerk/test_exceptions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,44 @@ def assert_error_message(message, error)
assert_equal message, error.message.lines.first.chomp
end

test "raises NameError if the expected constant is not defined" do
test "raises NameError if the expected constant is not defined (top-level)" do
on_teardown { remove_const :TyPo }

files = [["typo.rb", "TyPo = 1"]]
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 constant Typo, but didn't", error
assert_error_message "expected file #{typo_rb} to define the top-level constant Typo", error
assert_equal :Typo, error.name
end
end

test "eager loading raises NameError if files do not define the expected constants" do
test "raises NameError if the expected constant is not defined (namespace)" do
files = [["m/typo.rb", "M::TyPo = 1"]]
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_equal :Typo, error.name
end
end

test "eager loading raises NameError if files do not define the expected constants (top-level)" do
files = [["x.rb", ""]]
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 constant X, but didn't", error
assert_error_message "expected file #{x_rb} to define the top-level constant X", error
assert_equal :X, error.name
end
end

test "eager loading raises NameError if files do not define the expected constants (namespace)" do
files = [["m/x.rb", ""]]
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_equal :X, error.name
end
end
Expand All @@ -44,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 constant Cli::X, but didn't", error
assert_error_message "expected file #{cli_x_rb} to define the constant Cli::X", 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 constant X, but didn't/) do
assert_logged(/expected file #{File.expand_path("x.rb")} to define the top-level constant X/) do
loader.push_dir(".")
loader.setup
assert_raises(Zeitwerk::NameError) { X }
Expand Down

0 comments on commit 1671229

Please sign in to comment.