diff --git a/Gemfile.lock b/Gemfile.lock index 5552fb26b..fc3f9332d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -96,6 +96,7 @@ PATH parallel parser sorbet-runtime (>= 0.5.9914) + zeitwerk GEM remote: https://rubygems.org/ @@ -258,7 +259,6 @@ DEPENDENCIES sorbet-runtime spring tapioca - zeitwerk BUNDLED WITH 2.3.5 diff --git a/README.md b/README.md index ae09c3c58..b5771897b 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ We are keeping `packwerk` compatible with current versions of Ruby and Rails, bu -- _Sandi Metz, Practical Object-Oriented Design in Ruby_ -Packwerk is a Ruby gem used to enforce boundaries and modularize Rails applications. +Packwerk is a Ruby gem used to enforce boundaries and modularize Ruby applications (including Rails apps!). Packwerk can be used to: * Combine groups of files into packages diff --git a/lib/packwerk/application_load_paths.rb b/lib/packwerk/application_load_paths.rb index 51c26b2d3..2d642cb76 100644 --- a/lib/packwerk/application_load_paths.rb +++ b/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::Hash[String, Module]) } + sig { params(root: String, environment: String).returns(T::Array[String]) } def extract_relevant_paths(root, environment) require_application(root, environment) all_paths = extract_application_autoload_paths @@ -18,30 +18,28 @@ def extract_relevant_paths(root, environment) relative_path_strings(relevant_paths) end - sig { returns(T::Hash[String, Module]) } + sig { returns(T::Array[String]) } def extract_application_autoload_paths - Rails.autoloaders.inject({}) do |h, loader| - h.merge(loader.root_dirs) - end + Zeitwerk::Loader.all_dirs end sig do - params(all_paths: T::Hash[String, Module], bundle_path: Pathname, rails_root: Pathname) - .returns(T::Hash[Pathname, Module]) + params(all_paths: T::Array[String], bundle_path: Pathname, rails_root: Pathname) + .returns(T::Array[Pathname]) 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 } + .map { |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]) } + sig { params(load_paths: T::Array[Pathname], rails_root: Pathname).returns(T::Array[String]) } 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 } + load_paths.map { |path| Pathname.new(path).relative_path_from(rails_root).to_s } end private @@ -59,7 +57,7 @@ def require_application(root, environment) end end - sig { params(paths: T::Hash[T.untyped, Module]).void } + sig { params(paths: T::Array[Pathname]).void } def assert_load_paths_present(paths) if paths.empty? raise <<~EOS diff --git a/lib/packwerk/run_context.rb b/lib/packwerk/run_context.rb index 43a3b088c..29e514676 100644 --- a/lib/packwerk/run_context.rb +++ b/lib/packwerk/run_context.rb @@ -38,7 +38,7 @@ def from_configuration(configuration) sig do params( root_path: String, - load_paths: T::Hash[String, Module], + load_paths: T::Array[String], inflector: T.class_of(ActiveSupport::Inflector), cache_directory: Pathname, config_path: T.nilable(String), diff --git a/packwerk.gemspec b/packwerk.gemspec index 7cad32c06..327c8f1a7 100644 --- a/packwerk.gemspec +++ b/packwerk.gemspec @@ -45,13 +45,13 @@ Gem::Specification.new do |spec| spec.add_dependency("constant_resolver", ">=0.2.0") spec.add_dependency("parallel") spec.add_dependency("sorbet-runtime", ">=0.5.9914") + spec.add_dependency("zeitwerk") spec.add_development_dependency("m") spec.add_development_dependency("rake") spec.add_development_dependency("sorbet") # https://github.com/ruby/psych/pull/487 spec.add_development_dependency("psych", "~> 3") - spec.add_development_dependency("zeitwerk") # For Ruby parsing spec.add_dependency("ast") diff --git a/test/fixtures/skeleton/gems/gem_a/.gitkeep b/test/fixtures/skeleton/gems/gem_a/.gitkeep new file mode 100644 index 000000000..e69de29bb diff --git a/test/unit/application_load_paths_test.rb b/test/unit/application_load_paths_test.rb index 810bf3f22..e7ef3b012 100644 --- a/test/unit/application_load_paths_test.rb +++ b/test/unit/application_load_paths_test.rb @@ -11,44 +11,70 @@ class ApplicationLoadPathsTest < Minitest::Test relative_path = "app/models" absolute_path = rails_root.join(relative_path) relative_path_strings = ApplicationLoadPaths.relative_path_strings( - { absolute_path => Object }, + [absolute_path], rails_root: rails_root ) - assert_equal({ relative_path => Object }, relative_path_strings) + assert_equal([relative_path], 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 } + # paths = paths.each_with_object([]) { |p, h| h << p.to_s } filtered_paths = ApplicationLoadPaths.filter_relevant_paths( paths, bundle_path: Pathname.new("/application/vendor/"), rails_root: Pathname.new("/application/") ) - assert_equal({ "/application/app/models" => Object }, filtered_paths.transform_keys(&:to_s)) + assert_equal(["/application/app/models"], filtered_paths.map(&: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 } + # 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" => Object }, filtered_paths.transform_keys(&:to_s)) + assert_equal(["/application/app/models"], filtered_paths.map(&: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 => Object) + ApplicationLoadPaths.expects(:filter_relevant_paths).once.returns([Pathname.new("/fake_path").to_s]) ApplicationLoadPaths.expects(:require_application).with("/application", "test").once.returns(true) ApplicationLoadPaths.extract_relevant_paths("/application", "test") end + + test ".extract_relevant_paths uses Zeitwerk loaders" do + result = ApplicationLoadPaths.extract_relevant_paths("./test/fixtures/skeleton", "test") + assert_equal([ + "components/sales/app/models", + "components/platform/app/models", + "components/timeline/app/models", + "components/timeline/app/models/concerns", + "vendor/cache/gems/example/models", + ], + result) + + loader = Zeitwerk::Loader.new + loader.push_dir("./test/fixtures/skeleton/gems/gem_a") + + result = ApplicationLoadPaths.extract_relevant_paths("./test/fixtures/skeleton", "test") + assert_equal([ + "components/sales/app/models", + "components/platform/app/models", + "components/timeline/app/models", + "components/timeline/app/models/concerns", + "vendor/cache/gems/example/models", + "gems/gem_a", + ], + result) + end end end diff --git a/test/unit/cli_test.rb b/test/unit/cli_test.rb index ec5ee270a..18bf71635 100644 --- a/test/unit/cli_test.rb +++ b/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])