diff --git a/lib/zeitwerk/loader.rb b/lib/zeitwerk/loader.rb index 2e5533a6..6554affb 100644 --- a/lib/zeitwerk/loader.rb +++ b/lib/zeitwerk/loader.rb @@ -237,15 +237,13 @@ def cpath_expected_at(path) return unless dir?(abspath) || ruby?(abspath) return if ignored_path?(abspath) - cnames = [] - abspaths = [] + paths = [] if ruby?(abspath) basename = File.basename(abspath, ".rb") return if hidden?(basename) - cnames << inflector.camelize(basename, abspath).to_sym - abspaths << abspath + paths << [basename, abspath] walk_up_from = File.dirname(abspath) else walk_up_from = abspath @@ -260,28 +258,15 @@ def cpath_expected_at(path) basename = File.basename(dir) return if hidden?(basename) - unless collapse?(dir) - cnames << inflector.camelize(basename, dir).to_sym - abspaths << dir - end + paths << [basename, abspath] unless collapse?(dir) end return unless root_namespace - if cnames.empty? + if paths.empty? real_mod_name(root_namespace) else - # We reverse before validating the segments to report the leftmost - # problematic one, if any. - cnames.reverse! - - cname_validator = Module.new - cnames.each_with_index do |cname, i| - cname_validator.const_defined?(cname, false) - rescue ::NameError - j = -(i + 1) - raise Zeitwerk::NameError.new("cannot derive a constant name from #{abspaths[j]}") - end + cnames = paths.reverse_each.map { |b, a| cname_for(b, a) } if root_namespace == Object cnames.join("::") @@ -422,34 +407,15 @@ def all_dirs # @sig (String, Module) -> void private def set_autoloads_in_dir(dir, parent) ls(dir) do |basename, abspath| - begin - if ruby?(basename) - basename.delete_suffix!(".rb") - cname = inflector.camelize(basename, abspath).to_sym - autoload_file(parent, cname, abspath) + if ruby?(basename) + basename.delete_suffix!(".rb") + autoload_file(parent, cname_for(basename, abspath), abspath) + else + if collapse?(abspath) + set_autoloads_in_dir(abspath, parent) else - if collapse?(abspath) - set_autoloads_in_dir(abspath, parent) - else - cname = inflector.camelize(basename, abspath).to_sym - autoload_subdir(parent, cname, abspath) - end + autoload_subdir(parent, cname_for(basename, abspath), abspath) end - rescue ::NameError => error - path_type = ruby?(abspath) ? "file" : "directory" - - raise NameError.new(<<~MESSAGE, error.name) - #{error.message} inferred by #{inflector.class} from #{path_type} - - #{abspath} - - Possible ways to address this: - - * Tell Zeitwerk to ignore this particular #{path_type}. - * Tell Zeitwerk to ignore one of its parent directories. - * Rename the #{path_type} to comply with the naming conventions. - * Modify the inflector to handle this case. - MESSAGE end end end diff --git a/lib/zeitwerk/loader/helpers.rb b/lib/zeitwerk/loader/helpers.rb index 566eb821..52c0cf50 100644 --- a/lib/zeitwerk/loader/helpers.rb +++ b/lib/zeitwerk/loader/helpers.rb @@ -140,4 +140,48 @@ module Zeitwerk::Loader::Helpers private def crem(parent, cname) parent.__send__(:remove_const, cname) end + + CNAME_VALIDATOR = Module.new + private_constant :CNAME_VALIDATOR + + # @raise [Zeitwerk::NameError] + # @sig (String, String) -> Symbol + private def cname_for(basename, abspath) + cname = inflector.camelize(basename, abspath) + + unless cname.is_a?(String) + raise TypeError, "#{inflector.class}#camelize must return a String, received #{cname.inspect}" + end + + if cname.include?("::") + raise Zeitwerk::NameError.new(<<~MESSAGE, cname) + wrong constant name #{cname} inferred by #{inflector.class} from + + #{abspath} + + #{inflector.class}#camelize should return a simple constant name without "::" + MESSAGE + end + + begin + CNAME_VALIDATOR.const_defined?(cname, false) + rescue ::NameError => error + path_type = ruby?(abspath) ? "file" : "directory" + + raise Zeitwerk::NameError.new(<<~MESSAGE, error.name) + #{error.message} inferred by #{inflector.class} from #{path_type} + + #{abspath} + + Possible ways to address this: + + * Tell Zeitwerk to ignore this particular #{path_type}. + * Tell Zeitwerk to ignore one of its parent directories. + * Rename the #{path_type} to comply with the naming conventions. + * Modify the inflector to handle this case. + MESSAGE + end + + cname.to_sym + end end diff --git a/test/lib/test_custom_inflector.rb b/test/lib/test_custom_inflector.rb new file mode 100644 index 00000000..c041bfca --- /dev/null +++ b/test/lib/test_custom_inflector.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require "test_helper" + +class TestCustomInflector < LoaderTest + test "raises TypeError if the inflector #camelize does not return a string" do + with_files([["foo.rb", nil]]) do + loader.inflector = Class.new(Zeitwerk::Inflector) do + def camelize(_basename, _abspath) + :Foo # this is wrong + end + end.new + + loader.push_dir(".") + + error = assert_raises TypeError do + loader.setup + end + + assert_includes error.message, "#camelize must return a String" + end + end + + test "raises if the returned constant names has ::" do + with_files([["foo-bar.rb", nil]]) do + loader.inflector = Class.new(Zeitwerk::Inflector) do + def camelize(basename, _abspath) + if basename == "foo-bar" + "Foo::Bar" # this is wrong + else + super + end + end + end.new + + loader.push_dir(".") + + error = assert_raises Zeitwerk::NameError do + loader.setup + end + + assert_includes error.message, "wrong constant name Foo::Bar" + assert_includes error.message, "#camelize should return a simple constant name" + end + end +end diff --git a/test/lib/zeitwerk/test_expected_cpath_at.rb b/test/lib/zeitwerk/test_expected_cpath_at.rb index 741ab75a..4bdb61a4 100644 --- a/test/lib/zeitwerk/test_expected_cpath_at.rb +++ b/test/lib/zeitwerk/test_expected_cpath_at.rb @@ -19,13 +19,15 @@ class TestExpectedCpathAtErrors < LoaderTest with_files(files) do loader.push_dir(".") - files.each do |file, _contents| - error = assert_raises Zeitwerk::NameError do - loader.cpath_expected_at(file) - end - abspath = File.expand_path(file) - assert_includes error.message, "cannot derive a constant name from #{abspath}" + error = assert_raises Zeitwerk::NameError do + loader.cpath_expected_at(files[0][0]) + end + assert_includes error.message, "wrong constant name Foo-bar" + + error = assert_raises Zeitwerk::NameError do + loader.cpath_expected_at(files[1][0]) end + assert_includes error.message, "wrong constant name 1" end end @@ -35,8 +37,7 @@ class TestExpectedCpathAtErrors < LoaderTest error = assert_raises Zeitwerk::NameError do loader.cpath_expected_at("x/foo-bar/y/z.rb") end - abspath = File.expand_path("x/foo-bar") - assert_includes error.message, "cannot derive a constant name from #{abspath}" + assert_includes error.message, "wrong constant name Foo-bar" end end end diff --git a/test/lib/zeitwerk/test_ruby_compatibility.rb b/test/lib/zeitwerk/test_ruby_compatibility.rb index 79bc707d..e68eea7b 100644 --- a/test/lib/zeitwerk/test_ruby_compatibility.rb +++ b/test/lib/zeitwerk/test_ruby_compatibility.rb @@ -157,6 +157,15 @@ class C; end end end + # We delegate constant name validation to Module#const_defined?. + test "const_defined? raises NameError for invalid cnames" do + error = assert_raises ::NameError do + Module.new.const_defined?("Foo-Bar", false) + end + + assert_includes error.message, "wrong constant name Foo-Bar" + end + # Unloading removes autoloads by calling remove_const. It is convenient that # remove_const does not execute the autoload because it would be surprising, # and slower, that those unused files got loaded precisely while unloading.