Skip to content

Commit d5b07fa

Browse files
AndrewSwerlickAndrewSwerlick
committed
Refactor out package scanning into PackagePaths
Moves arguments passed into the self methods on `PackageSet` into a new `PackagePaths` object. This makes it easier to add new arguments without alot of parameter drilling. This will support the next commit where we add the ability to conditionally find packages outside of the app dir
1 parent 57c8dcd commit d5b07fa

File tree

9 files changed

+172
-105
lines changed

9 files changed

+172
-105
lines changed

‎lib/packwerk.rb‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ module Packwerk
3434
autoload :OutputStyle
3535
autoload :Package
3636
autoload :PackageSet
37+
autoload :PackagePaths
3738
autoload :ParsedConstantDefinitions
3839
autoload :Parsers
3940
autoload :ParseRun

‎lib/packwerk/application_validator.rb‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -280,8 +280,12 @@ def package_glob
280280

281281
sig { params(glob_pattern: T.any(T::Array[String], String)).returns(T::Array[String]) }
282282
def package_manifests(glob_pattern = package_glob)
283-
PackageSet.package_paths(@configuration.root_path, glob_pattern, @configuration.exclude)
284-
.map { |f| File.realpath(f) }
283+
package_paths = PackagePaths.new(
284+
@configuration.root_path,
285+
glob_pattern,
286+
@configuration.exclude,
287+
)
288+
package_paths.all_paths.map { |f| File.realpath(f) }
285289
end
286290

287291
sig { params(paths: T::Array[String]).returns(T::Array[Pathname]) }
@@ -299,7 +303,7 @@ def invalid_package_path?(path)
299303
# Packages at the root can be implicitly specified as "."
300304
return false if path == "."
301305

302-
package_path = File.join(@configuration.root_path, path, PackageSet::PACKAGE_CONFIG_FILENAME)
306+
package_path = File.join(@configuration.root_path, path, PackagePaths::PACKAGE_CONFIG_FILENAME)
303307
!File.file?(package_path)
304308
end
305309

‎lib/packwerk/configuration.rb‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,13 @@ def from_packwerk_config(path)
3434
DEFAULT_EXCLUDE_GLOBS = ["{bin,node_modules,script,tmp,vendor}/**/*"]
3535

3636
attr_reader(
37-
:include, :exclude, :root_path, :package_paths, :custom_associations, :config_path, :cache_directory
37+
:include,
38+
:exclude,
39+
:root_path,
40+
:package_paths,
41+
:custom_associations,
42+
:config_path,
43+
:cache_directory,
3844
)
3945

4046
def initialize(configs = {}, config_path: nil)

‎lib/packwerk/package_paths.rb‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
# typed: strict
2+
# frozen_string_literal: true
3+
4+
require "pathname"
5+
require "bundler"
6+
7+
module Packwerk
8+
class PackagePaths
9+
PACKAGE_CONFIG_FILENAME = "package.yml"
10+
PathSpec = T.type_alias { T.any(String, T::Array[String]) }
11+
12+
extend T::Sig
13+
extend T::Generic
14+
15+
sig do
16+
params(
17+
root_path: String,
18+
package_pathspec: T.nilable(PathSpec),
19+
exclude_pathspec: T.nilable(PathSpec),
20+
).void
21+
end
22+
def initialize(root_path, package_pathspec, exclude_pathspec = nil)
23+
@root_path = root_path
24+
@package_pathspec = package_pathspec
25+
@exclude_pathspec = exclude_pathspec
26+
end
27+
28+
def all_paths
29+
exclude_pathspec = Array(@exclude_pathspec).dup
30+
.push(Bundler.bundle_path.join("**").to_s)
31+
.map { |glob| File.expand_path(glob) }
32+
33+
paths_to_scan = [@root_path]
34+
35+
glob_patterns = paths_to_scan.product(Array(@package_pathspec)).map do |path, pathspec|
36+
File.join(path, pathspec, PACKAGE_CONFIG_FILENAME)
37+
end
38+
39+
Dir.glob(glob_patterns)
40+
.map { |path| Pathname.new(path).cleanpath }
41+
.reject { |path| exclude_path?(exclude_pathspec, path) }
42+
end
43+
44+
private
45+
46+
sig { params(globs: T::Array[String], path: Pathname).returns(T::Boolean) }
47+
def exclude_path?(globs, path)
48+
globs.any? do |glob|
49+
path.realpath.fnmatch(glob, File::FNM_EXTGLOB)
50+
end
51+
end
52+
end
53+
end

‎lib/packwerk/package_set.rb‎

Lines changed: 6 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
require "bundler"
66

77
module Packwerk
8-
PathSpec = T.type_alias { T.any(String, T::Array[String]) }
98

109
# A set of {Packwerk::Package}s as well as methods to parse packages from the filesystem.
1110
class PackageSet
@@ -15,16 +14,17 @@ class PackageSet
1514

1615
Elem = type_member { { fixed: Package } }
1716

18-
PACKAGE_CONFIG_FILENAME = "package.yml"
1917

2018
class << self
2119
extend T::Sig
2220

23-
sig { params(root_path: String, package_pathspec: T.nilable(PathSpec)).returns(PackageSet) }
21+
sig do
22+
params(root_path: String, package_pathspec: T.nilable(PackagePaths::PathSpec)).returns(PackageSet)
23+
end
2424
def load_all_from(root_path, package_pathspec: nil)
25-
package_paths = package_paths(root_path, package_pathspec || "**")
25+
package_paths = PackagePaths.new(root_path, package_pathspec || "**", nil)
2626

27-
packages = package_paths.map do |path|
27+
packages = package_paths.all_paths.map do |path|
2828
root_relative = path.dirname.relative_path_from(root_path)
2929
Package.new(name: root_relative.to_s, config: YAML.load_file(path))
3030
end
@@ -34,27 +34,6 @@ def load_all_from(root_path, package_pathspec: nil)
3434
new(packages)
3535
end
3636

37-
sig do
38-
params(
39-
root_path: String,
40-
package_pathspec: PathSpec,
41-
exclude_pathspec: T.nilable(PathSpec)
42-
).returns(T::Array[Pathname])
43-
end
44-
def package_paths(root_path, package_pathspec, exclude_pathspec = [])
45-
exclude_pathspec = Array(exclude_pathspec).dup
46-
.push(Bundler.bundle_path.join("**").to_s)
47-
.map { |glob| File.expand_path(glob) }
48-
49-
glob_patterns = Array(package_pathspec).map do |pathspec|
50-
File.join(root_path, pathspec, PACKAGE_CONFIG_FILENAME)
51-
end
52-
53-
Dir.glob(glob_patterns)
54-
.map { |path| Pathname.new(path).cleanpath }
55-
.reject { |path| exclude_path?(exclude_pathspec, path) }
56-
end
57-
5837
private
5938

6039
sig { params(packages: T::Array[Package]).void }
@@ -64,12 +43,7 @@ def create_root_package_if_none_in(packages)
6443
packages << Package.new(name: Package::ROOT_PACKAGE_NAME, config: nil)
6544
end
6645

67-
sig { params(globs: T::Array[String], path: Pathname).returns(T::Boolean) }
68-
def exclude_path?(globs, path)
69-
globs.any? do |glob|
70-
path.realpath.fnmatch(glob, File::FNM_EXTGLOB)
71-
end
72-
end
46+
7347
end
7448

7549
sig { returns(T::Hash[String, Package]) }

‎lib/packwerk/run_context.rb‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@ def initialize(
6868
@cache_enabled = cache_enabled
6969
@cache_directory = cache_directory
7070
@config_path = config_path
71-
7271
@file_processor = T.let(nil, T.nilable(FileProcessor))
7372
@context_provider = T.let(nil, T.nilable(ConstantDiscovery))
7473
# We need to initialize this before we fork the process, see https://github.com/Shopify/packwerk/issues/182
@@ -125,7 +124,10 @@ def resolver
125124

126125
sig { returns(PackageSet) }
127126
def package_set
128-
::Packwerk::PackageSet.load_all_from(@root_path, package_pathspec: @package_paths)
127+
::Packwerk::PackageSet.load_all_from(
128+
@root_path,
129+
package_pathspec: @package_paths,
130+
)
129131
end
130132

131133
sig { returns(T::Array[ConstantNameInspector]) }

‎test/unit/application_validator_test.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ class ApplicationValidatorTest < Minitest::Test
160160
merge_into_app_yaml_file("packwerk.yml", { "package_paths" => ["components/**/*", "."] })
161161
merge_into_app_yaml_file("packwerk.yml", { "exclude" => ["vendor/**/*"] })
162162

163-
package_paths = PackageSet.package_paths(".", "**")
163+
package_paths = PackagePaths.new(".", "**").all_paths
164164
vendor_package_path = Pathname.new("vendor/cache/gems/example/package.yml")
165165
assert_includes(package_paths, vendor_package_path)
166166

‎test/unit/package_paths_test.rb‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
# typed: true
2+
# frozen_string_literal: true
3+
4+
require "test_helper"
5+
6+
module Packwerk
7+
class PackagePathsTest < Minitest::Test
8+
include RailsApplicationFixtureHelper
9+
10+
setup do
11+
setup_application_fixture
12+
end
13+
14+
teardown do
15+
teardown_application_fixture
16+
end
17+
18+
test "#all_paths supports a path wildcard" do
19+
use_template(:skeleton)
20+
package_paths = PackagePaths.new(".", "**")
21+
22+
assert_includes(package_paths.all_paths, Pathname.new("components/sales/package.yml"))
23+
assert_includes(package_paths.all_paths, Pathname.new("package.yml"))
24+
end
25+
26+
test "#all_paths supports a single path as a string" do
27+
use_template(:skeleton)
28+
package_paths = PackagePaths.new(".", "components/sales")
29+
30+
assert_equal(package_paths.all_paths, [Pathname.new("components/sales/package.yml")])
31+
end
32+
33+
test "#all_paths supports many paths as an array" do
34+
use_template(:skeleton)
35+
package_paths = PackagePaths.new(".", ["components/sales", "."])
36+
37+
assert_equal(
38+
package_paths.all_paths,
39+
[
40+
Pathname.new("components/sales/package.yml"),
41+
Pathname.new("package.yml"),
42+
]
43+
)
44+
end
45+
46+
test "#all_paths excludes paths inside the gem directory" do
47+
use_template(:skeleton)
48+
vendor_package_path = Pathname.new("vendor/cache/gems/example/package.yml")
49+
50+
package_paths = PackagePaths.new(".", "**")
51+
assert_includes(package_paths.all_paths, vendor_package_path)
52+
53+
Bundler.expects(:bundle_path).once.returns(Rails.root.join("vendor/cache/gems"))
54+
package_paths = PackagePaths.new(".", "**")
55+
refute_includes(package_paths.all_paths, vendor_package_path)
56+
end
57+
58+
test "#all_paths excludes paths in exclude pathspec" do
59+
use_template(:skeleton)
60+
vendor_package_path = Pathname.new("vendor/cache/gems/example/package.yml")
61+
62+
package_paths = PackagePaths.new(".", "**")
63+
assert_includes(package_paths.all_paths, vendor_package_path)
64+
65+
package_paths = PackagePaths.new(".", "**", "vendor/*")
66+
refute_includes(package_paths.all_paths, vendor_package_path)
67+
end
68+
69+
test "#all_paths excludes paths in multiple exclude pathspecs" do
70+
use_template(:skeleton)
71+
72+
vendor_package_path = Pathname.new("vendor/cache/gems/example/package.yml")
73+
sales_package_path = Pathname.new("components/sales/package.yml")
74+
75+
package_paths = PackagePaths.new(".", "**")
76+
assert_includes(package_paths.all_paths, vendor_package_path)
77+
assert_includes(package_paths.all_paths, sales_package_path)
78+
79+
package_paths = PackagePaths.new(".", "**", ["vendor/*", "components/sales/*"])
80+
refute_includes(package_paths.all_paths, vendor_package_path)
81+
refute_includes(package_paths.all_paths, sales_package_path)
82+
end
83+
84+
test "#all_paths ignores empty excludes" do
85+
use_template(:skeleton)
86+
87+
assert_equal(
88+
PackagePaths.new(".", "**").all_paths,
89+
PackagePaths.new(".", "**", []).all_paths
90+
)
91+
end
92+
end
93+
end

‎test/unit/package_set_test.rb‎

Lines changed: 0 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -39,71 +39,5 @@ class PackageSetTest < Minitest::Test
3939
test "#fetch returns nil for unknown package name" do
4040
assert_nil(@package_set.fetch("components/unknown"))
4141
end
42-
43-
test ".package_paths supports a path wildcard" do
44-
package_paths = PackageSet.package_paths(".", "**")
45-
46-
assert_includes(package_paths, Pathname.new("components/sales/package.yml"))
47-
assert_includes(package_paths, Pathname.new("package.yml"))
48-
end
49-
50-
test ".package_paths supports a single path as a string" do
51-
package_paths = PackageSet.package_paths(".", "components/sales")
52-
53-
assert_equal(package_paths, [Pathname.new("components/sales/package.yml")])
54-
end
55-
56-
test ".package_paths supports many paths as an array" do
57-
package_paths = PackageSet.package_paths(".", ["components/sales", "."])
58-
59-
assert_equal(
60-
package_paths,
61-
[
62-
Pathname.new("components/sales/package.yml"),
63-
Pathname.new("package.yml"),
64-
]
65-
)
66-
end
67-
68-
test ".package_paths excludes paths inside the gem directory" do
69-
vendor_package_path = Pathname.new("vendor/cache/gems/example/package.yml")
70-
71-
package_paths = PackageSet.package_paths(".", "**")
72-
assert_includes(package_paths, vendor_package_path)
73-
74-
Bundler.expects(:bundle_path).returns(Rails.root.join("vendor/cache/gems"))
75-
package_paths = PackageSet.package_paths(".", "**")
76-
refute_includes(package_paths, vendor_package_path)
77-
end
78-
79-
test ".package_paths excludes paths in exclude pathspec" do
80-
vendor_package_path = Pathname.new("vendor/cache/gems/example/package.yml")
81-
82-
package_paths = PackageSet.package_paths(".", "**")
83-
assert_includes(package_paths, vendor_package_path)
84-
85-
package_paths = PackageSet.package_paths(".", "**", "vendor/*")
86-
refute_includes(package_paths, vendor_package_path)
87-
end
88-
89-
test ".package_paths excludes paths in multiple exclude pathspecs" do
90-
vendor_package_path = Pathname.new("vendor/cache/gems/example/package.yml")
91-
sales_package_path = Pathname.new("components/sales/package.yml")
92-
93-
package_paths = PackageSet.package_paths(".", "**")
94-
assert_includes(package_paths, vendor_package_path)
95-
assert_includes(package_paths, sales_package_path)
96-
97-
package_paths = PackageSet.package_paths(".", "**", ["vendor/*", "components/sales/*"])
98-
refute_includes(package_paths, vendor_package_path)
99-
refute_includes(package_paths, sales_package_path)
100-
end
101-
102-
test ".package_paths ignores empty excludes" do
103-
assert_equal(
104-
PackageSet.package_paths(".", "**"),
105-
PackageSet.package_paths(".", "**", [])
106-
)
107-
end
10842
end
10943
end

0 commit comments

Comments
 (0)