Merge pull request #224 from Shopify/revert-220-cleaner_signatures_for_load_paths

Revert "Simplify packwerk path determination mechanics"

Author: rafaelfranca

lib/packwerk/application_load_paths.rb

@@ -9,7 +9,7 @@ module ApplicationLoadPaths
     class << self
       extend T::Sig
 
-      sig { params(root: String, environment: String).returns(T::Array[String]) }
+      sig { params(root: String, environment: String).returns(T::Hash[String, Module]) }
       def extract_relevant_paths(root, environment)
         require_application(root, environment)
         all_paths = extract_application_autoload_paths
@@ -18,30 +18,30 @@ def extract_relevant_paths(root, environment)
         relative_path_strings(relevant_paths)
       end
 
-      sig { returns(T::Array[String]) }
+      sig { returns(T::Hash[String, Module]) }
       def extract_application_autoload_paths
         Rails.autoloaders.inject({}) do |h, loader|
           h.merge(loader.root_dirs)
-        end.keys
+        end
       end
 
       sig do
-        params(all_paths: T::Array[String], bundle_path: Pathname, rails_root: Pathname)
-          .returns(T::Array[Pathname])
+        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
-          .map { |path| Pathname.new(path).expand_path }
+          .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::Array[Pathname], rails_root: Pathname).returns(T::Array[String]) }
+      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.map { |path| Pathname.new(path).relative_path_from(rails_root).to_s }
+        load_paths.transform_keys { |path| Pathname.new(path).relative_path_from(rails_root).to_s }
       end
 
       private
@@ -59,7 +59,7 @@ def require_application(root, environment)
         end
       end
 
-      sig { params(paths: T::Array[Pathname]).void }
+      sig { params(paths: T::Hash[T.untyped, Module]).void }
       def assert_load_paths_present(paths)
         if paths.empty?
           raise <<~EOS

lib/packwerk/run_context.rb

@@ -38,7 +38,7 @@ def from_configuration(configuration)
     sig do
       params(
         root_path: String,
-        load_paths: T::Array[String],
+        load_paths: T::Hash[String, Module],
         inflector: T.class_of(ActiveSupport::Inflector),
         cache_directory: Pathname,
         config_path: T.nilable(String),

test/unit/application_load_paths_test.rb

@@ -11,39 +11,41 @@ class ApplicationLoadPathsTest < Minitest::Test
       relative_path = "app/models"
       absolute_path = rails_root.join(relative_path)
       relative_path_strings = ApplicationLoadPaths.relative_path_strings(
-        [absolute_path],
+        { absolute_path => Object },
         rails_root: rails_root
       )
 
-      assert_equal([relative_path], relative_path_strings)
+      assert_equal({ relative_path => Object }, relative_path_strings)
     end
 
     test ".filter_relevant_paths excludes paths outside of the application root" do
       valid_paths = ["/application/app/models"]
       paths = valid_paths + ["/users/tobi/.gems/something/app/models", "/application/../something/"]
+      paths = paths.each_with_object({}) { |p, h| h[p.to_s] = Object }
       filtered_paths = ApplicationLoadPaths.filter_relevant_paths(
         paths,
         bundle_path: Pathname.new("/application/vendor/"),
         rails_root: Pathname.new("/application/")
       )
 
-      assert_equal(["/application/app/models"], filtered_paths.map(&:to_s))
+      assert_equal({ "/application/app/models" => Object }, filtered_paths.transform_keys(&:to_s))
     end
 
     test ".filter_relevant_paths excludes paths from vendored gems" do
       valid_paths = ["/application/app/models"]
       paths = valid_paths + ["/application/vendor/something/app/models"]
+      paths = paths.each_with_object({}) { |p, h| h[p.to_s] = Object }
       filtered_paths = ApplicationLoadPaths.filter_relevant_paths(
         paths,
         bundle_path: Pathname.new("/application/vendor/"),
         rails_root: Pathname.new("/application/")
       )
 
-      assert_equal(["/application/app/models"], filtered_paths.map(&:to_s))
+      assert_equal({ "/application/app/models" => Object }, filtered_paths.transform_keys(&:to_s))
     end
 
     test ".extract_relevant_paths calls out to filter the paths" do
-      ApplicationLoadPaths.expects(:filter_relevant_paths).once.returns([Pathname.new("/fake_path").to_s])
+      ApplicationLoadPaths.expects(:filter_relevant_paths).once.returns(Pathname.new("/fake_path").to_s => Object)
       ApplicationLoadPaths.expects(:require_application).with("/application", "test").once.returns(true)
 
       ApplicationLoadPaths.extract_relevant_paths("/application", "test")

test/unit/cli_test.rb

@@ -24,7 +24,7 @@ class CliTest < Minitest::Test
       offense = Offense.new(file: file_path, message: violation_message)
 
       configuration = Configuration.new({ "parallel" => false })
-      configuration.stubs(load_paths: [])
+      configuration.stubs(load_paths: {})
       RunContext.any_instance.stubs(:process_file).at_least_once.returns([offense])
 
       string_io = StringIO.new
@@ -49,7 +49,7 @@ class CliTest < Minitest::Test
       offense = Offense.new(file: file_path, message: violation_message)
 
       configuration = Configuration.new({ "parallel" => false })
-      configuration.stubs(load_paths: [])
+      configuration.stubs(load_paths: {})
 
       RunContext.any_instance.stubs(:process_file)
         .at_least(2)
@@ -137,7 +137,7 @@ def show_stale_violations(_offense_collection, _fileset)
       offense = Offense.new(file: file_path, message: violation_message)
 
       configuration = Configuration.new
-      configuration.stubs(load_paths: [])
+      configuration.stubs(load_paths: {})
       RunContext.any_instance.stubs(:process_file)
         .returns([offense])