Skip to content

Commit

Permalink
add support for specifying default root namespaces
Browse files Browse the repository at this point in the history
This changes a couple things when running Packwerk against a Rails app.
First of all, instead of getting our load paths via
`config.autoload_paths` and related methods, we use Rails' zeitwerk
autoloaders. Secondly, instead of representing the load paths as a list
of root directory paths, we instead use a hashmap of root directory
paths to module instances which are the root modules for the path.
This allows users to override the default module for any root directory.
  • Loading branch information
mclark authored Apr 13, 2022
1 parent b5effdf commit 1f9df95
Show file tree
Hide file tree
Showing 13 changed files with 83 additions and 122 deletions.
3 changes: 2 additions & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ DEPENDENCIES
sorbet-runtime
spring
tapioca
zeitwerk

BUNDLED WITH
2.3.4
2.3.5
30 changes: 12 additions & 18 deletions lib/packwerk/application_load_paths.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -18,36 +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.application.railties
.select { |railtie| railtie.is_a?(Rails::Engine) }
.push(Rails.application)
.flat_map do |engine|
paths = (engine.config.autoload_paths + engine.config.eager_load_paths + engine.config.autoload_once_paths)
paths.map(&:to_s).uniq
end
Rails.autoloaders.inject({}) do |h, loader|
h.merge(loader.root_dirs)
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(paths: T::Array[Pathname], rails_root: Pathname).returns(T::Array[String]) }
def relative_path_strings(paths, rails_root: Rails.root)
paths
.map { |path| path.relative_path_from(rails_root).to_s }
.uniq
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

private
Expand All @@ -65,7 +59,7 @@ def require_application(root, environment)
end
end

sig { params(paths: T::Array[T.untyped]).void }
sig { params(paths: T::Hash[T.untyped, Module]).void }
def assert_load_paths_present(paths)
if paths.empty?
raise <<~EOS
Expand Down
2 changes: 1 addition & 1 deletion lib/packwerk/run_context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions packwerk.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ Gem::Specification.new do |spec|
spec.add_development_dependency("m")
# 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")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# typed: ignore
# frozen_string_literal: true

module Company
class Special
end
end
8 changes: 4 additions & 4 deletions test/integration/custom_executable_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ class CustomExecutableTest < Minitest::Test

deprecated_reference_content_after_update = read_deprecated_references
expected_output = <<~EOS
📦 Packwerk is inspecting 12 files
\\.\\.\\.\\.\\.\\.\\.\\.\\.\\.\\.\\.
📦 Packwerk is inspecting 13 files
\\.\\.\\.\\.\\.\\.\\.\\.\\.\\.\\.\\.\\.
📦 Finished in \\d+\\.\\d+ seconds
No offenses detected
Expand Down Expand Up @@ -115,8 +115,8 @@ class CustomExecutableTest < Minitest::Test
deprecated_reference_content_after_update =
read_deprecated_references.reject { |k, _v| k.match?(timeline_deprecated_reference_path) }
expected_output = <<~EOS
📦 Packwerk is inspecting 13 files
\\.\\.\\.\\.\\.\\.\\.\\.\\.\\.\\.\\.\\.
📦 Packwerk is inspecting 14 files
\\.\\.\\.\\.\\.\\.\\.\\.\\.\\.\\.\\.\\.\\.
📦 Finished in \\d+\\.\\d+ seconds
No offenses detected
Expand Down
5 changes: 5 additions & 0 deletions test/rails_test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ def self.skeleton(*path)
ROOT.join("test", "fixtures", "skeleton", *path).to_s
end

config.autoloader = :zeitwerk
config.eager_load = false
config.eager_load_paths = [skeleton("components", "platform", "app", "models")]
config.autoload_paths = [skeleton("components", "sales", "app", "models")]
config.autoload_once_paths = [
Expand All @@ -16,4 +18,7 @@ def self.skeleton(*path)
skeleton("vendor", "cache", "gems", "example", "models"),
]
config.root = skeleton(".")
config.logger = Logger.new("/dev/null")
end

Rails.application.initialize!
57 changes: 28 additions & 29 deletions test/support/rails_application_fixture_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,36 @@
# frozen_string_literal: true

require "rails_test_helper"
require "zeitwerk"

module RailsApplicationFixtureHelper
include ApplicationFixtureHelper

def setup_application_fixture
super()
cache_rails_paths
end
class Autoloaders
include Enumerable

def initialize
@main = Zeitwerk::Loader.new
@once = Zeitwerk::Loader.new
end

attr_reader :main, :once

def each(&block)
block.call(main)
block.call(once)
end

def teardown_application_fixture
restore_rails_paths
super()
def zeitwerk_enabled?
true
end
end

def use_template(template)
super(template)

Rails.stubs(:autoloaders).returns(Autoloaders.new)

case template
when :minimal
set_load_paths_for_minimal_template
Expand All @@ -28,36 +41,22 @@ def use_template(template)
raise "Unknown fixture template #{template}"
end

RailsPaths.root(app_dir)
root = Pathname.new(app_dir)
Rails.application.config.stubs(:root).returns(root)
end

private

def set_load_paths_for_minimal_template
RailsPaths.autoload(to_app_paths("/components/sales/app/models"))
RailsPaths.eager_load([])
RailsPaths.autoload_once([])
Rails.autoloaders.main.push_dir(*to_app_paths("/components/sales/app/models"))
end

def set_load_paths_for_skeleton_template
RailsPaths.autoload(to_app_paths("/components/sales/app/models"))
RailsPaths.eager_load(to_app_paths("components/platform/app/models"))
RailsPaths.autoload_once(
to_app_paths(
"components/timeline/app/models",
"components/timeline/app/models/concerns",
"vendor/cache/gems/example/models",
)
)
end

def cache_rails_paths
raise "cache_rails_paths may only be called once per test" if defined? @rails_paths
@rails_paths = RailsPaths.new
@rails_paths.cache
end
Rails.autoloaders.main.push_dir(*to_app_paths("/components/sales/app/models"))
Rails.autoloaders.main.push_dir(*to_app_paths("components/platform/app/models"))

def restore_rails_paths
@rails_paths.restore
Rails.autoloaders.once.push_dir(*to_app_paths("components/timeline/app/models"))
Rails.autoloaders.once.push_dir(*to_app_paths("components/timeline/app/models/concerns"))
Rails.autoloaders.once.push_dir(*to_app_paths("vendor/cache/gems/example/models"))
end
end
36 changes: 0 additions & 36 deletions test/support/rails_paths.rb

This file was deleted.

1 change: 0 additions & 1 deletion test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
require "support/application_fixture_helper"
require "support/factory_helper"
require "support/rails_application_fixture_helper"
require "support/rails_paths"
require "support/test_macro"
require "support/test_assertions"
require "support/yaml_file"
Expand Down
36 changes: 7 additions & 29 deletions test/unit/application_load_paths_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,66 +11,44 @@ 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 valid_paths, 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 valid_paths, 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")])
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")
end

test ".extract_relevant_paths returns unique load paths" do
path = Pathname.new("/application/app/models")
ApplicationLoadPaths.expects(:filter_relevant_paths).once.returns([path, path])
ApplicationLoadPaths.expects(:require_application).with("/application", "test").once.returns(true)

assert_equal 1, ApplicationLoadPaths.extract_relevant_paths("/application", "test").count
end

test ".extract_application_autoload_paths returns unique autoload paths" do
path = Pathname.new("/application/app/models")
Rails.application.config.expects(:autoload_paths).once.returns([path])
Rails.application.config.expects(:eager_load_paths).once.returns([path])
Rails.application.config.expects(:autoload_once_paths).once.returns([path])

assert_equal 1, ApplicationLoadPaths.extract_application_autoload_paths.count
end

test ".extract_application_autoload_paths returns autoload paths as strings" do
path = Pathname.new("/application/app/models")
Rails.application.config.expects(:autoload_paths).once.returns([path])

assert_instance_of String, ApplicationLoadPaths.extract_application_autoload_paths.first
end
end
end
6 changes: 3 additions & 3 deletions test/unit/cli_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -137,7 +137,7 @@ def show_stale_violations(_offense_collection)
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])

Expand Down
Loading

0 comments on commit 1f9df95

Please sign in to comment.