Proof of concept for removing ConstantResolver and using Zeitwerk for ConstantDiscovery

Gemfile

@@ -8,7 +8,6 @@ gemspec
 # Specify the same dependency sources as the application Gemfile
 
 gem("spring")
-gem("constant_resolver", require: false)
 gem("rubocop-performance", require: false)
 gem("rubocop-sorbet", require: false)
 gem("mocha", require: false)

Gemfile.lock

@@ -6,12 +6,11 @@ PATH
       ast
       better_html
       bundler
-      constant_resolver (>= 0.2.0)
       parallel
       parser
       prism (>= 0.25.0)
       sorbet-runtime (>= 0.5.9914)
-      zeitwerk (>= 2.6.1)
+      zeitwerk (>= 2.6.14)
 
 GEM
   remote: https://rubygems.org/
@@ -45,7 +44,6 @@ GEM
     builder (3.2.4)
     byebug (11.1.3)
     concurrent-ruby (1.1.10)
-    constant_resolver (0.2.0)
     crass (1.0.6)
     erubi (1.11.0)
     i18n (1.12.0)
@@ -156,19 +154,19 @@ GEM
     yard-sorbet (0.8.1)
       sorbet-runtime (>= 0.5)
       yard (>= 0.9)
-    zeitwerk (2.6.4)
+    zeitwerk (2.6.16)
 
 PLATFORMS
   aarch64-linux
   arm64-darwin-21
   arm64-darwin-22
+  arm64-darwin-23
   x86_64-darwin
   x86_64-darwin-20
   x86_64-linux
 
 DEPENDENCIES
   byebug
-  constant_resolver
   m
   minitest-focus
   mocha

lib/packwerk.rb

@@ -4,6 +4,7 @@
 require "sorbet-runtime"
 require "active_support"
 require "fileutils"
+require "zeitwerk"
 
 # Provides String#pluralize
 require "active_support/core_ext/string"

lib/packwerk/application_validator.rb

@@ -1,7 +1,6 @@
 # typed: strict
 # frozen_string_literal: true
 
-require "constant_resolver"
 require "pathname"
 require "yaml"
 
@@ -25,7 +24,7 @@ def check_all(package_set, configuration)
     def call(package_set, configuration)
       results = [
         check_package_manifest_syntax(configuration),
-        check_application_structure(configuration),
+        check_application_structure(configuration, packages: package_set),
         check_package_manifest_paths(configuration),
         check_root_package_exists(configuration),
       ]
@@ -68,15 +67,16 @@ def check_package_manifest_syntax(configuration)
       end
     end
 
-    sig { params(configuration: Configuration).returns(Validator::Result) }
-    def check_application_structure(configuration)
-      resolver = ConstantResolver.new(
+    sig { params(configuration: Configuration, packages: PackageSet).returns(Validator::Result) }
+    def check_application_structure(configuration, packages:)
+      constant_discovery = ConstantDiscovery.new(
+        packages,
         root_path: configuration.root_path.to_s,
-        load_paths: configuration.load_paths
+        loaders:   configuration.loaders
       )
 
       begin
-        resolver.file_map
+        constant_discovery.validate_constants
         Validator::Result.new(ok: true)
       rescue => e
         Validator::Result.new(ok: false, error_value: e.message)

lib/packwerk/configuration.rb

@@ -96,12 +96,9 @@ def initialize(configs = {}, config_path: nil)
       end
     end
 
-    sig { returns(T::Hash[String, Module]) }
-    def load_paths
-      @load_paths ||= T.let(
-        RailsLoadPaths.for(@root_path, environment: "test"),
-        T.nilable(T::Hash[String, Module]),
-      )
+    sig { returns(T::Enumerable[Zeitwerk::Loader]) }
+    def loaders
+      @loaders ||= RailsLoadPaths.loaders_for(@root_path, environment: "test")
     end
 
     sig { returns(T::Boolean) }

lib/packwerk/constant_discovery.rb

@@ -1,30 +1,25 @@
 # typed: strict
 # frozen_string_literal: true
 
-require "constant_resolver"
-
 module Packwerk
-  # Get information about unresolved constants without loading the application code.
-  # Information gathered: Fully qualified name, path to file containing the definition, package,
-  # and visibility (public/private to the package).
-  #
-  # The implementation makes a few assumptions about the code base:
-  # - `Something::SomeOtherThing` is defined in a path of either `something/some_other_thing.rb` or `something.rb`,
-  #   relative to the load path. Rails' `zeitwerk` autoloader makes the same assumption.
-  # - It is OK to not always infer the exact file defining the constant. For example, when a constant is inherited, we
-  #   have no way of inferring the file it is defined in. You could argue though that inheritance means that another
-  #   constant with the same name exists in the inheriting class, and this view is sufficient for all our use cases.
+  # Determines context for constants based on the provided `Zeitwerk::Loaders` such as
+  # their file location and the package they belong to.
   class ConstantDiscovery
     extend T::Sig
 
-    # @param constant_resolver [ConstantResolver]
-    # @param packages [Packwerk::PackageSet]
+    class Error < StandardError; end
+
     sig do
-      params(constant_resolver: ConstantResolver, packages: Packwerk::PackageSet).void
+      params(
+        packages:  PackageSet,
+        root_path: String,
+        loaders:   T::Enumerable[Zeitwerk::Loader]
+      ).void
     end
-    def initialize(constant_resolver:, packages:)
+    def initialize(packages, root_path:, loaders:)
       @packages = packages
-      @resolver = constant_resolver
+      @root_path = root_path
+      @loaders = loaders
     end
 
     # Get the package that owns a given file path.
@@ -56,20 +51,109 @@ def package_from_path(path)
       ).returns(T.nilable(ConstantContext))
     end
     def context_for(const_name, current_namespace_path: [])
-      begin
-        constant = @resolver.resolve(const_name, current_namespace_path: current_namespace_path)
-      rescue ConstantResolver::Error => e
-        raise(ConstantResolver::Error, e.message)
+      current_namespace_path = [] if const_name.start_with?("::")
+      const_name, location = resolve_constant(const_name.delete_prefix("::"), current_namespace_path)
+
+      return unless location
+
+      location = location.delete_prefix("#{@root_path}#{File::SEPARATOR}").to_s
+      ConstantContext.new(const_name, location, package_from_path(location))
+    end
+
+    # Analyze the constants and raise errors if any potential issues are encountered that would prevent
+    # resolving the context for constants, such as ambiguous constant locations.
+    #
+    # @return [ConstantDiscovery]
+    sig { returns(ConstantDiscovery) }
+    def validate_constants
+      tap { const_locations }
+    end
+
+    sig { returns(String) }
+    def inspect
+      "#<#{self.class.name}>"
+    end
+
+    private
+
+    sig { returns(T::Hash[String, String]) }
+    def const_locations
+      return @const_locations unless @const_locations.nil?
+
+      all_cpaths = @loaders.inject({}) do |cpaths, loader|
+        paths = loader.all_expected_cpaths.filter do |cpath, _const|
+          cpath.ends_with?(".rb")
+        end
+        cpaths.merge(paths)
       end
 
-      return unless constant
+      paths_by_const = all_cpaths.invert
+      validate_constant_paths(paths_by_const, all_cpaths: all_cpaths)
+      @const_locations = paths_by_const
+    end
 
-      package = @packages.package_from_path(constant.location)
-      ConstantContext.new(
-        constant.name,
-        constant.location,
-        package,
-      )
+    sig do
+      params(
+        const_name:             String,
+        current_namespace_path: T.nilable(T::Array[String]),
+        original_name:          String
+      ).returns(T::Array[T.nilable(String)])
+    end
+    def resolve_constant(const_name, current_namespace_path, original_name: const_name)
+      namespace, location = resolve_traversing_namespace_path(const_name, current_namespace_path)
+      if location
+        ["::" + namespace.push(original_name).join("::"), location]
+      elsif !const_name.include?("::")
+        # constant could not be resolved to a file in the given load paths
+        [nil, nil]
+      else
+        parent_constant = const_name.split("::")[0..-2].join("::")
+        resolve_constant(parent_constant, current_namespace_path, original_name: original_name)
+      end
+    end
+
+    sig do
+      params(
+        const_name:             String,
+        current_namespace_path: T.nilable(T::Array[String]),
+      ).returns(T::Array[T.nilable(String)])
+    end
+    def resolve_traversing_namespace_path(const_name, current_namespace_path)
+      fully_qualified_name_guess = (current_namespace_path + [const_name]).join("::")
+
+      location = const_locations[fully_qualified_name_guess]
+      if location || fully_qualified_name_guess == const_name
+        [current_namespace_path, location]
+      else
+        resolve_traversing_namespace_path(const_name, current_namespace_path[0..-2])
+      end
+    end
+
+    sig do
+      params(
+        paths_by_constant: T::Hash[String, String],
+        all_cpaths:        T::Hash[String, String]
+      ).void
+    end
+    def validate_constant_paths(paths_by_constant, all_cpaths:)
+      raise(Error, "Could not find any ruby files.") if all_cpaths.empty?
+
+      is_ambiguous = all_cpaths.size != paths_by_constant.size
+      raise(Error, ambiguous_constants_hint(paths_by_constant, all_cpaths: all_cpaths)) if is_ambiguous
+    end
+
+    sig do
+      params(
+        paths_by_constant: T::Hash[String, String],
+        all_cpaths:        T::Hash[String, String]
+      ).returns(String)
+    end
+    def ambiguous_constants_hint(paths_by_constant, all_cpaths:)
+      ambiguous_constants = all_cpaths.except(*paths_by_constant.invert.keys).values
+      <<~MSG
+        Ambiguous constant definition:
+        #{ambiguous_constants.map { |const| " - #{const}" }.join("\n")}
+      MSG
     end
   end
 

lib/packwerk/rails_load_paths.rb

@@ -11,43 +11,14 @@ module RailsLoadPaths
     class << self
       extend T::Sig
 
-      sig { params(root: String, environment: String).returns(T::Hash[String, Module]) }
-      def for(root, environment:)
+      sig { params(root: String, environment: String).returns(T::Enumerable[Zeitwerk::Loader]) }
+      def loaders_for(root, environment:)
         require_application(root, environment)
-        all_paths = extract_application_autoload_paths
-        relevant_paths = filter_relevant_paths(all_paths)
-        assert_load_paths_present(relevant_paths)
-        relative_path_strings(relevant_paths)
+        Rails.autoloaders
       end
 
       private
 
-      sig { returns(T::Hash[String, Module]) }
-      def extract_application_autoload_paths
-        Rails.autoloaders.inject({}) do |h, loader|
-          h.merge(loader.dirs(namespaces: true))
-        end
-      end
-
-      sig do
-        params(all_paths: T::Hash[String, Module], bundle_path: Pathname, rails_root: Pathname)
-          .returns(T::Hash[Pathname, Module])
-      end
-      def filter_relevant_paths(all_paths, bundle_path: Bundler.bundle_path, rails_root: Rails.root)
-        bundle_path_match = bundle_path.join("**")
-        rails_root_match = rails_root.join("**")
-
-        all_paths
-          .transform_keys { |path| Pathname.new(path).expand_path }
-          .select { |path| path.fnmatch(rails_root_match.to_s) } # path needs to be in application directory
-          .reject { |path| path.fnmatch(bundle_path_match.to_s) } # reject paths from vendored gems
-      end
-
-      sig { params(load_paths: T::Hash[Pathname, Module], rails_root: Pathname).returns(T::Hash[String, Module]) }
-      def relative_path_strings(load_paths, rails_root: Rails.root)
-        load_paths.transform_keys { |path| Pathname.new(path).relative_path_from(rails_root).to_s }
-      end
-
       sig { params(root: String, environment: String).void }
       def require_application(root, environment)
         environment_file = "#{root}/config/environment"
@@ -60,16 +31,6 @@ def require_application(root, environment)
           raise "A Rails application could not be found in #{root}"
         end
       end
-
-      sig { params(paths: T::Hash[T.untyped, Module]).void }
-      def assert_load_paths_present(paths)
-        if paths.empty?
-          raise <<~EOS
-            We could not extract autoload paths from your Rails app. This is likely a configuration error.
-            Packwerk will not work correctly without any autoload paths.
-          EOS
-        end
-      end
     end
   end
 end