Centralize and improve camelize validations

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

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

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

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

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.