From 7e348584a5794c80b3c1650ed9ac59d2e7cbba4b Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Tue, 6 Dec 2022 08:21:06 -0500 Subject: [PATCH 01/16] Mark Packwerk::Generators as private --- lib/packwerk.rb | 2 ++ lib/packwerk/cli.rb | 4 ++-- test/unit/generators/configuration_file_test.rb | 4 ++-- test/unit/generators/root_package_test.rb | 4 ++-- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/packwerk.rb b/lib/packwerk.rb index da4033c11..dedcf97ef 100644 --- a/lib/packwerk.rb +++ b/lib/packwerk.rb @@ -76,6 +76,8 @@ module Generators autoload :RootPackage end + private_constant :Generators + module ReferenceChecking extend ActiveSupport::Autoload diff --git a/lib/packwerk/cli.rb b/lib/packwerk/cli.rb index 34ac173b7..5487d69e6 100644 --- a/lib/packwerk/cli.rb +++ b/lib/packwerk/cli.rb @@ -77,12 +77,12 @@ def init sig { returns(T::Boolean) } def generate_configs - configuration_file = Packwerk::Generators::ConfigurationFile.generate( + configuration_file = Generators::ConfigurationFile.generate( root: @configuration.root_path, out: @out ) - root_package = Packwerk::Generators::RootPackage.generate(root: @configuration.root_path, out: @out) + root_package = Generators::RootPackage.generate(root: @configuration.root_path, out: @out) success = configuration_file && root_package diff --git a/test/unit/generators/configuration_file_test.rb b/test/unit/generators/configuration_file_test.rb index d31dee7bf..31dd10afe 100644 --- a/test/unit/generators/configuration_file_test.rb +++ b/test/unit/generators/configuration_file_test.rb @@ -17,14 +17,14 @@ class ConfigurationFileTest < Minitest::Test test ".generate creates a default configuration file if there were empty load paths array" do generated_file_path = File.join(@temp_dir, Packwerk::Configuration::DEFAULT_CONFIG_PATH) - assert(Packwerk::Generators::ConfigurationFile.generate(root: @temp_dir, out: @string_io)) + assert(Generators::ConfigurationFile.generate(root: @temp_dir, out: @string_io)) assert(File.exist?(generated_file_path)) end test ".generate does not create a configuration file if a file exists" do file_path = File.join(@temp_dir, Packwerk::Configuration::DEFAULT_CONFIG_PATH) File.open(file_path, "w") do |_f| - assert(Packwerk::Generators::ConfigurationFile.generate( + assert(Generators::ConfigurationFile.generate( root: @temp_dir, out: @string_io )) diff --git a/test/unit/generators/root_package_test.rb b/test/unit/generators/root_package_test.rb index c1ce57c7c..af4aae126 100644 --- a/test/unit/generators/root_package_test.rb +++ b/test/unit/generators/root_package_test.rb @@ -17,7 +17,7 @@ class RootPackageTest < Minitest::Test end test ".generate creates a package.yml file" do - success = Packwerk::Generators::RootPackage.generate(root: @temp_dir, out: @string_io) + success = Generators::RootPackage.generate(root: @temp_dir, out: @string_io) assert(File.exist?(@generated_file_path)) assert success assert_includes @string_io.string, "root package generated" @@ -25,7 +25,7 @@ class RootPackageTest < Minitest::Test test ".generate does not create a package.yml file if package.yml already exists" do File.open(File.join(@temp_dir, "package.yml"), "w") do |_f| - success = Packwerk::Generators::RootPackage.generate(root: @temp_dir, out: @string_io) + success = Generators::RootPackage.generate(root: @temp_dir, out: @string_io) assert success assert_includes @string_io.string, "Root package already exists" end From 42ab000911033f958a47e8e5210ac5077ea82258 Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Tue, 6 Dec 2022 08:21:54 -0500 Subject: [PATCH 02/16] Mark Packwerk::ReferenceChecking as private --- lib/packwerk.rb | 2 ++ test/unit/checker_test.rb | 2 +- test/unit/offense_collection_test.rb | 2 +- test/unit/package_todo_test.rb | 18 +++++++++--------- test/unit/parse_run_test.rb | 6 +++--- .../reference_checker_test.rb | 2 +- 6 files changed, 17 insertions(+), 15 deletions(-) diff --git a/lib/packwerk.rb b/lib/packwerk.rb index dedcf97ef..a2b4d544d 100644 --- a/lib/packwerk.rb +++ b/lib/packwerk.rb @@ -91,6 +91,8 @@ module Checkers end end + private_constant :ReferenceChecking + class ApplicationValidator extend ActiveSupport::Autoload diff --git a/test/unit/checker_test.rb b/test/unit/checker_test.rb index 971a6c5c6..1d5ab8e03 100644 --- a/test/unit/checker_test.rb +++ b/test/unit/checker_test.rb @@ -7,7 +7,7 @@ module Packwerk class CheckerTest < Minitest::Test test "#find is correctly able to find the right checker" do found_checker = Checker.find("dependency") - assert T.unsafe(found_checker).is_a?(Packwerk::ReferenceChecking::Checkers::DependencyChecker) + assert T.unsafe(found_checker).is_a?(ReferenceChecking::Checkers::DependencyChecker) end end end diff --git a/test/unit/offense_collection_test.rb b/test/unit/offense_collection_test.rb index fdfbe38b0..ae0dc2256 100644 --- a/test/unit/offense_collection_test.rb +++ b/test/unit/offense_collection_test.rb @@ -51,7 +51,7 @@ class OffenseCollectionTest < Minitest::Test package_todo = Packwerk::PackageTodo.new(package, ".") package_todo .stubs(:listed?) - .with(reference, violation_type: Packwerk::ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE) + .with(reference, violation_type: ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE) .returns(true) Packwerk::PackageTodo .stubs(:new) diff --git a/test/unit/package_todo_test.rb b/test/unit/package_todo_test.rb index 994772eff..36a7107ad 100644 --- a/test/unit/package_todo_test.rb +++ b/test/unit/package_todo_test.rb @@ -91,9 +91,9 @@ class PackageTodoTest < Minitest::Test package_todo = PackageTodo.new(package, "test/fixtures/package_todo.yml") package_todo.add_entries(first_violated_reference, - Packwerk::ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE) + ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE) package_todo.add_entries(second_violated_reference, - Packwerk::ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE) + ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE) assert package_todo.stale_violations?(Set.new([ "orders/app/jobs/orders/sweepers/purge_old_document_rows_task.rb", "orders/app/models/orders/services/adjustment_engine.rb", @@ -152,7 +152,7 @@ class PackageTodoTest < Minitest::Test package_todo = PackageTodo.new(reference.constant.package, file.path) package_todo.add_entries(reference, - Packwerk::ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE) + ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE) package_todo.dump expected_output = { @@ -206,17 +206,17 @@ class PackageTodoTest < Minitest::Test ) package_todo.add_entries(second_package_first_reference, - Packwerk::ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE) + ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE) package_todo.add_entries(second_package_first_reference, - Packwerk::ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE) + ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE) package_todo.add_entries(second_package_second_reference, - Packwerk::ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE) + ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE) package_todo.add_entries(second_package_second_reference, - Packwerk::ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE) + ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE) package_todo.add_entries(second_package_third_reference, - Packwerk::ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE) + ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE) package_todo.add_entries(first_package_reference, - Packwerk::ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE) + ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE) package_todo.dump diff --git a/test/unit/parse_run_test.rb b/test/unit/parse_run_test.rb index 2c8a21c5e..2155b7982 100644 --- a/test/unit/parse_run_test.rb +++ b/test/unit/parse_run_test.rb @@ -312,7 +312,7 @@ def something offense = ReferenceOffense.new( reference: build_reference(path: file_to_check), message: "some message", - violation_type: Packwerk::ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE + violation_type: ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE ) RunContext.any_instance.stubs(:process_file).returns([offense]) @@ -359,7 +359,7 @@ def something offense = ReferenceOffense.new( reference: build_reference(path: "components/source/some/path.rb", source_package: source_package), message: "some message", - violation_type: Packwerk::ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE + violation_type: ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE ) RunContext.any_instance.stubs(:process_file).returns([offense]) @@ -415,7 +415,7 @@ def something offense = ReferenceOffense.new( reference: build_reference(path: file_to_check), message: "some message", - violation_type: Packwerk::ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE + violation_type: ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE ) RunContext.any_instance.stubs(:process_file).returns([offense]) diff --git a/test/unit/reference_checking/reference_checker_test.rb b/test/unit/reference_checking/reference_checker_test.rb index 7342a3a61..c0339f579 100644 --- a/test/unit/reference_checking/reference_checker_test.rb +++ b/test/unit/reference_checking/reference_checker_test.rb @@ -52,7 +52,7 @@ def strict_mode_violation?(_listed_offense) end def reference_checker(checkers = [StubChecker.new]) - Packwerk::ReferenceChecking::ReferenceChecker.new(checkers) + ReferenceChecking::ReferenceChecker.new(checkers) end end end From 327724e80bfe6a1d7a4ea886a7f7abd9f9f81830 Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Tue, 6 Dec 2022 08:23:03 -0500 Subject: [PATCH 03/16] Mark Packwerk::Formatters as private --- USAGE.md | 2 +- lib/packwerk.rb | 2 ++ test/unit/parse_run_test.rb | 14 +++++++------- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/USAGE.md b/USAGE.md index 58b87d5f3..688c68433 100644 --- a/USAGE.md +++ b/USAGE.md @@ -265,7 +265,7 @@ You can also reference the name of a gem. While `packwerk` ships with its own offense formatter, you may specify a custom one in your configuration file via the `offenses_formatter:` key. Your custom formatter will be used when `bin/packwerk check` is run. -Firstly, you'll need to create an `OffensesFormatter` class that includes `Packwerk::OffensesFormatter`. You can use [`Packwerk::Formatters::OffensesFormatter`](lib/packwerk/formatters/offenses_formatter.rb) as a point of reference for this. Then, in the `require` directive described above, you'll want to tell `packwerk` about it: +Firstly, you'll need to create an `OffensesFormatter` class that includes `Packwerk::OffensesFormatter`. You can use [`Formatters::OffensesFormatter`](lib/packwerk/formatters/offenses_formatter.rb) as a point of reference for this. Then, in the `require` directive described above, you'll want to tell `packwerk` about it: ```ruby # ./path/to/file.rb class MyOffensesFormatter diff --git a/lib/packwerk.rb b/lib/packwerk.rb index a2b4d544d..577f132eb 100644 --- a/lib/packwerk.rb +++ b/lib/packwerk.rb @@ -69,6 +69,8 @@ module Formatters autoload :ProgressFormatter end + private_constant :Formatters + module Generators extend ActiveSupport::Autoload diff --git a/test/unit/parse_run_test.rb b/test/unit/parse_run_test.rb index 2155b7982..eb967ff84 100644 --- a/test/unit/parse_run_test.rb +++ b/test/unit/parse_run_test.rb @@ -136,7 +136,7 @@ def something parse_run = Packwerk::ParseRun.new( relative_file_set: Set.new(["some/path.rb"]), configuration: Configuration.new({ "parallel" => false }), - progress_formatter: Packwerk::Formatters::ProgressFormatter.new(out) + progress_formatter: Formatters::ProgressFormatter.new(out) ) RunContext.any_instance.stubs(:process_file).returns([offense]) result = parse_run.check @@ -213,7 +213,7 @@ def something parse_run = Packwerk::ParseRun.new( relative_file_set: Set.new([file_to_check]), configuration: Configuration.new({ "parallel" => false }), - progress_formatter: Packwerk::Formatters::ProgressFormatter.new(out) + progress_formatter: Formatters::ProgressFormatter.new(out) ) RunContext.any_instance.stubs(:process_file).returns([offense1, offense2]) result = parse_run.check @@ -259,7 +259,7 @@ def something parse_run = Packwerk::ParseRun.new( relative_file_set: Set.new([file_to_check]), configuration: Configuration.new({ "parallel" => false }), - progress_formatter: Packwerk::Formatters::ProgressFormatter.new(out) + progress_formatter: Formatters::ProgressFormatter.new(out) ) RunContext.any_instance.stubs(:process_file).returns([]) result = parse_run.check @@ -306,7 +306,7 @@ def something parse_run = Packwerk::ParseRun.new( relative_file_set: Set.new([file_to_check]), configuration: Configuration.new({ "parallel" => false }), - progress_formatter: Packwerk::Formatters::ProgressFormatter.new(out) + progress_formatter: Formatters::ProgressFormatter.new(out) ) offense = ReferenceOffense.new( @@ -353,7 +353,7 @@ def something parse_run = Packwerk::ParseRun.new( relative_file_set: Set.new(["components/source/some/path.rb"]), configuration: Configuration.new({ "parallel" => false }), - progress_formatter: Packwerk::Formatters::ProgressFormatter.new(out) + progress_formatter: Formatters::ProgressFormatter.new(out) ) offense = ReferenceOffense.new( @@ -409,7 +409,7 @@ def something parse_run = Packwerk::ParseRun.new( relative_file_set: Set.new([file_to_check]), configuration: Configuration.new({ "parallel" => false, "offenses_formatter" => "plain" }), - progress_formatter: Packwerk::Formatters::ProgressFormatter.new(out) + progress_formatter: Formatters::ProgressFormatter.new(out) ) offense = ReferenceOffense.new( @@ -457,7 +457,7 @@ def something parse_run = Packwerk::ParseRun.new( relative_file_set: Set.new(["some/path.rb"]), configuration: Configuration.new({ "parallel" => false }), - progress_formatter: Packwerk::Formatters::ProgressFormatter.new(out) + progress_formatter: Formatters::ProgressFormatter.new(out) ) RunContext.any_instance.stubs(:process_file).returns([offense]) result = parse_run.check From 6cc6dd03c868c246f2037699c1afbb3a3ff37f22 Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Tue, 6 Dec 2022 08:26:06 -0500 Subject: [PATCH 04/16] sort autoload list --- lib/packwerk.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/packwerk.rb b/lib/packwerk.rb index 577f132eb..1dab26ca6 100644 --- a/lib/packwerk.rb +++ b/lib/packwerk.rb @@ -16,7 +16,6 @@ module Packwerk autoload :ApplicationLoadPaths autoload :ApplicationValidator autoload :AssociationInspector - autoload :OffenseCollection autoload :Cache autoload :Checker autoload :Cli @@ -24,7 +23,6 @@ module Packwerk autoload :ConstantDiscovery autoload :ConstantNameInspector autoload :ConstNodeInspector - autoload :PackageTodo autoload :ExtensionLoader autoload :FileProcessor autoload :FilesForProcessing @@ -36,19 +34,21 @@ module Packwerk autoload :NodeProcessorFactory autoload :NodeVisitor autoload :Offense + autoload :OffenseCollection autoload :OffensesFormatter autoload :OutputStyle autoload :Package autoload :PackageSet + autoload :PackageTodo autoload :ParsedConstantDefinitions autoload :Parsers autoload :ParseRun - autoload :UnresolvedReference autoload :Reference autoload :ReferenceExtractor autoload :ReferenceOffense autoload :Result autoload :RunContext + autoload :UnresolvedReference autoload :Validator autoload :Version From e874d0bbf07a6acda470b79dfe38f488b61cd3c5 Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Tue, 6 Dec 2022 08:37:54 -0500 Subject: [PATCH 05/16] Mark API as private and reference it in permitted way --- lib/packwerk.rb | 58 +++++-- lib/packwerk/cli.rb | 6 +- lib/packwerk/configuration.rb | 2 +- lib/packwerk/constant_discovery.rb | 2 +- lib/packwerk/file_processor.rb | 2 +- ...atter.rb => default_offenses_formatter.rb} | 6 +- lib/packwerk/generators/configuration_file.rb | 2 +- lib/packwerk/node_processor_factory.rb | 8 +- lib/packwerk/offenses_formatter.rb | 2 +- lib/packwerk/parse_run.rb | 6 +- lib/packwerk/reference_extractor.rb | 2 +- lib/packwerk/run_context.rb | 6 +- test/parser_test_helper.rb | 10 +- test/support/application_fixture_helper.rb | 154 +++++++++--------- test/support/factory_helper.rb | 38 +++-- test/support/offenses_formatter_plain.rb | 4 +- .../rails_application_fixture_helper.rb | 2 +- test/unit/application_validator_test.rb | 4 +- test/unit/cache_test.rb | 2 +- test/unit/cli_test.rb | 4 +- test/unit/file_processor_test.rb | 4 +- test/unit/files_for_processing_test.rb | 20 +-- .../generators/configuration_file_test.rb | 4 +- test/unit/node_visitor_test.rb | 2 +- test/unit/parse_run_test.rb | 24 +-- test/unit/reference_extractor_test.rb | 8 +- 26 files changed, 209 insertions(+), 173 deletions(-) rename lib/packwerk/formatters/{offenses_formatter.rb => default_offenses_formatter.rb} (92%) diff --git a/lib/packwerk.rb b/lib/packwerk.rb index 1dab26ca6..ed6530c75 100644 --- a/lib/packwerk.rb +++ b/lib/packwerk.rb @@ -13,44 +13,74 @@ module Packwerk extend ActiveSupport::Autoload + # Public APIs + autoload :Checker + autoload :Cli + autoload :Offense + autoload :OffensesFormatter + autoload :OutputStyle + autoload :Package + autoload :PackageSet + autoload :PackageTodo + autoload :Parsers + autoload :Reference + autoload :ReferenceOffense + autoload :Result + + # Private APIs + # Please submit an issue if you have a use-case for these autoload :ApplicationLoadPaths + private_constant :ApplicationLoadPaths autoload :ApplicationValidator + private_constant :ApplicationValidator autoload :AssociationInspector + private_constant :AssociationInspector autoload :Cache - autoload :Checker - autoload :Cli + private_constant :Cache autoload :Configuration + private_constant :Configuration autoload :ConstantDiscovery + private_constant :ConstantDiscovery autoload :ConstantNameInspector + private_constant :ConstantNameInspector autoload :ConstNodeInspector + private_constant :ConstNodeInspector autoload :ExtensionLoader + private_constant :ExtensionLoader autoload :FileProcessor + private_constant :FileProcessor autoload :FilesForProcessing + private_constant :FilesForProcessing autoload :Graph + private_constant :Graph autoload :Loader + private_constant :Loader autoload :Node + private_constant :Node autoload :NodeHelpers + private_constant :NodeHelpers autoload :NodeProcessor + private_constant :NodeProcessor autoload :NodeProcessorFactory + private_constant :NodeProcessorFactory autoload :NodeVisitor - autoload :Offense + private_constant :NodeVisitor autoload :OffenseCollection - autoload :OffensesFormatter - autoload :OutputStyle - autoload :Package - autoload :PackageSet - autoload :PackageTodo + private_constant :OffenseCollection autoload :ParsedConstantDefinitions - autoload :Parsers + private_constant :ParsedConstantDefinitions autoload :ParseRun - autoload :Reference + private_constant :ParseRun autoload :ReferenceExtractor - autoload :ReferenceOffense - autoload :Result + private_constant :ReferenceExtractor autoload :RunContext + private_constant :RunContext autoload :UnresolvedReference + private_constant :UnresolvedReference autoload :Validator + private_constant :Validator autoload :Version + private_constant :Version module OutputStyles extend ActiveSupport::Autoload @@ -103,9 +133,9 @@ class ApplicationValidator end end -# Required to register the default OffensesFormatter +# Required to register the DefaultOffensesFormatter # We put this at the *end* of the file to specify all autoloads first -require "packwerk/formatters/offenses_formatter" +require "packwerk/formatters/default_offenses_formatter" # Required to register the default DependencyChecker require "packwerk/reference_checking/checkers/dependency_checker" diff --git a/lib/packwerk/cli.rb b/lib/packwerk/cli.rb index 5487d69e6..0d86c73cc 100644 --- a/lib/packwerk/cli.rb +++ b/lib/packwerk/cli.rb @@ -14,8 +14,8 @@ class Cli out: T.any(StringIO, IO), err_out: T.any(StringIO, IO), environment: String, - style: Packwerk::OutputStyle, - offenses_formatter: T.nilable(Packwerk::OffensesFormatter) + style: OutputStyle, + offenses_formatter: T.nilable(OffensesFormatter) ).void end def initialize( @@ -159,7 +159,7 @@ def validate(_paths) sig { returns(ApplicationValidator) } def validator - Packwerk::ApplicationValidator.new + ApplicationValidator.new end sig { returns(PackageSet) } diff --git a/lib/packwerk/configuration.rb b/lib/packwerk/configuration.rb index a1ba82cc2..9b96ed453 100644 --- a/lib/packwerk/configuration.rb +++ b/lib/packwerk/configuration.rb @@ -51,7 +51,7 @@ def initialize(configs = {}, config_path: nil) @cache_directory = Pathname.new(configs["cache_directory"] || "tmp/cache/packwerk") @config_path = config_path - @offenses_formatter_identifier = configs["offenses_formatter"] || Formatters::OffensesFormatter::IDENTIFIER + @offenses_formatter_identifier = configs["offenses_formatter"] || Formatters::DefaultOffensesFormatter::IDENTIFIER if configs.key?("require") configs["require"].each do |require_directive| diff --git a/lib/packwerk/constant_discovery.rb b/lib/packwerk/constant_discovery.rb index 8890b35a9..18be4afc0 100644 --- a/lib/packwerk/constant_discovery.rb +++ b/lib/packwerk/constant_discovery.rb @@ -50,7 +50,7 @@ def package_from_path(path) # @param const_name [String] The unresolved constant's name. # @param current_namespace_path [Array] (optional) The namespace of the context in which the constant is # used, e.g. ["Apps", "Models"] for `Apps::Models`. Defaults to [] which means top level. - # @return [Packwerk::ConstantDiscovery::ConstantContext] + # @return [ConstantDiscovery::ConstantContext] sig do params( const_name: String, diff --git a/lib/packwerk/file_processor.rb b/lib/packwerk/file_processor.rb index 6f9452499..6f2d4d961 100644 --- a/lib/packwerk/file_processor.rb +++ b/lib/packwerk/file_processor.rb @@ -64,7 +64,7 @@ def references_from_ast(node, relative_file) references = [] node_processor = @node_processor_factory.for(relative_file: relative_file, node: node) - node_visitor = Packwerk::NodeVisitor.new(node_processor: node_processor) + node_visitor = NodeVisitor.new(node_processor: node_processor) node_visitor.visit(node, ancestors: [], result: references) references diff --git a/lib/packwerk/formatters/offenses_formatter.rb b/lib/packwerk/formatters/default_offenses_formatter.rb similarity index 92% rename from lib/packwerk/formatters/offenses_formatter.rb rename to lib/packwerk/formatters/default_offenses_formatter.rb index e5f331b5e..5fca60a96 100644 --- a/lib/packwerk/formatters/offenses_formatter.rb +++ b/lib/packwerk/formatters/default_offenses_formatter.rb @@ -3,8 +3,8 @@ module Packwerk module Formatters - class OffensesFormatter - include Packwerk::OffensesFormatter + class DefaultOffensesFormatter + include OffensesFormatter IDENTIFIER = T.let("default", String) @@ -20,7 +20,7 @@ def show_offenses(offenses) EOS end - sig { override.params(offense_collection: Packwerk::OffenseCollection, fileset: T::Set[String]).returns(String) } + sig { override.params(offense_collection: OffenseCollection, fileset: T::Set[String]).returns(String) } def show_stale_violations(offense_collection, fileset) if offense_collection.stale_violations?(fileset) "There were stale violations found, please run `packwerk update-todo`" diff --git a/lib/packwerk/generators/configuration_file.rb b/lib/packwerk/generators/configuration_file.rb index 136440202..1f5dd948c 100644 --- a/lib/packwerk/generators/configuration_file.rb +++ b/lib/packwerk/generators/configuration_file.rb @@ -25,7 +25,7 @@ def initialize(root:, out: $stdout) sig { returns(T::Boolean) } def generate @out.puts("📦 Generating Packwerk configuration file...") - default_config_path = File.join(@root, ::Packwerk::Configuration::DEFAULT_CONFIG_PATH) + default_config_path = File.join(@root, Configuration::DEFAULT_CONFIG_PATH) if File.exist?(default_config_path) @out.puts("⚠️ Packwerk configuration file already exists.") diff --git a/lib/packwerk/node_processor_factory.rb b/lib/packwerk/node_processor_factory.rb index 111d0497d..e40ba884f 100644 --- a/lib/packwerk/node_processor_factory.rb +++ b/lib/packwerk/node_processor_factory.rb @@ -6,12 +6,12 @@ class NodeProcessorFactory < T::Struct extend T::Sig const :root_path, String - const :context_provider, Packwerk::ConstantDiscovery + const :context_provider, ConstantDiscovery const :constant_name_inspectors, T::Array[ConstantNameInspector] sig { params(relative_file: String, node: AST::Node).returns(NodeProcessor) } def for(relative_file:, node:) - ::Packwerk::NodeProcessor.new( + NodeProcessor.new( reference_extractor: reference_extractor(node: node), relative_file: relative_file, ) @@ -19,9 +19,9 @@ def for(relative_file:, node:) private - sig { params(node: AST::Node).returns(::Packwerk::ReferenceExtractor) } + sig { params(node: AST::Node).returns(ReferenceExtractor) } def reference_extractor(node:) - ::Packwerk::ReferenceExtractor.new( + ReferenceExtractor.new( constant_name_inspectors: constant_name_inspectors, root_node: node, root_path: root_path, diff --git a/lib/packwerk/offenses_formatter.rb b/lib/packwerk/offenses_formatter.rb index 46151c812..ca8e79209 100644 --- a/lib/packwerk/offenses_formatter.rb +++ b/lib/packwerk/offenses_formatter.rb @@ -61,7 +61,7 @@ def formatter_by_identifier(name) def show_offenses(offenses) end - sig { abstract.params(offense_collection: Packwerk::OffenseCollection, for_files: T::Set[String]).returns(String) } + sig { abstract.params(offense_collection: OffenseCollection, for_files: T::Set[String]).returns(String) } def show_stale_violations(offense_collection, for_files) end diff --git a/lib/packwerk/parse_run.rb b/lib/packwerk/parse_run.rb index d72f07335..f07d8d288 100644 --- a/lib/packwerk/parse_run.rb +++ b/lib/packwerk/parse_run.rb @@ -45,7 +45,7 @@ def update_todo return Result.new(message: message, status: false) end - run_context = Packwerk::RunContext.from_configuration(@configuration) + run_context = RunContext.from_configuration(@configuration) offense_collection = find_offenses(run_context) offense_collection.persist_package_todo_files(run_context.package_set) @@ -59,7 +59,7 @@ def update_todo sig { returns(Result) } def check - run_context = Packwerk::RunContext.from_configuration(@configuration) + run_context = RunContext.from_configuration(@configuration) offense_collection = find_offenses(run_context, show_errors: true) messages = [ @@ -76,7 +76,7 @@ def check private - sig { params(run_context: Packwerk::RunContext, show_errors: T::Boolean).returns(OffenseCollection) } + sig { params(run_context: RunContext, show_errors: T::Boolean).returns(OffenseCollection) } def find_offenses(run_context, show_errors: false) offense_collection = OffenseCollection.new(@configuration.root_path) all_offenses = T.let([], T::Array[Offense]) diff --git a/lib/packwerk/reference_extractor.rb b/lib/packwerk/reference_extractor.rb index 4e97b4aa1..18ec0d0a3 100644 --- a/lib/packwerk/reference_extractor.rb +++ b/lib/packwerk/reference_extractor.rb @@ -51,7 +51,7 @@ def get_fully_qualified_references_from(unresolved_references, context_provider) sig do params( - constant_name_inspectors: T::Array[Packwerk::ConstantNameInspector], + constant_name_inspectors: T::Array[ConstantNameInspector], root_node: ::AST::Node, root_path: String, ).void diff --git a/lib/packwerk/run_context.rb b/lib/packwerk/run_context.rb index 9fa6d6f6e..f04db76f5 100644 --- a/lib/packwerk/run_context.rb +++ b/lib/packwerk/run_context.rb @@ -109,7 +109,7 @@ def node_processor_factory sig { returns(ConstantDiscovery) } def context_provider - @context_provider ||= ::Packwerk::ConstantDiscovery.new( + @context_provider ||= ConstantDiscovery.new( constant_resolver: resolver, packages: package_set ) @@ -127,8 +127,8 @@ def resolver sig { returns(T::Array[ConstantNameInspector]) } def constant_name_inspectors [ - ::Packwerk::ConstNodeInspector.new, - ::Packwerk::AssociationInspector.new(inflector: @inflector, custom_associations: @custom_associations), + ConstNodeInspector.new, + AssociationInspector.new(inflector: @inflector, custom_associations: @custom_associations), ] end end diff --git a/test/parser_test_helper.rb b/test/parser_test_helper.rb index e41982d5c..d90423ec2 100644 --- a/test/parser_test_helper.rb +++ b/test/parser_test_helper.rb @@ -1,10 +1,12 @@ # typed: true # frozen_string_literal: true -module ParserTestHelper - class << self - def parse(source) - Packwerk::Parsers::Ruby.new.call(io: StringIO.new(source)) +module Packwerk + module ParserTestHelper + class << self + def parse(source) + Parsers::Ruby.new.call(io: StringIO.new(source)) + end end end end diff --git a/test/support/application_fixture_helper.rb b/test/support/application_fixture_helper.rb index 5cc652a96..bc0f6b96c 100644 --- a/test/support/application_fixture_helper.rb +++ b/test/support/application_fixture_helper.rb @@ -1,104 +1,106 @@ # typed: true # frozen_string_literal: true -module ApplicationFixtureHelper - TEMP_FIXTURE_DIR = ROOT.join("tmp", "fixtures").to_s - DEFAULT_TEMPLATE = :minimal +module Packwerk + module ApplicationFixtureHelper + TEMP_FIXTURE_DIR = ROOT.join("tmp", "fixtures").to_s + DEFAULT_TEMPLATE = :minimal - extend T::Helpers + extend T::Helpers - requires_ancestor { Kernel } + requires_ancestor { Kernel } - def setup_application_fixture - @old_working_dir = Dir.pwd - end + def setup_application_fixture + @old_working_dir = Dir.pwd + end - def remove_extensions - Object.send(:remove_const, :MyLocalExtension) - reset_formatters - end + def remove_extensions + Object.send(:remove_const, :MyLocalExtension) + reset_formatters + end - def reset_formatters - Packwerk::OffensesFormatter.instance_variable_set(:@formatter_by_identifier, nil) - current_formatters = Packwerk::OffensesFormatter.instance_variable_get(:@offenses_formatters) - new_formatters = current_formatters.delete_if { |f| f.new.identifier == "my_offenses_formatter" } - Packwerk::OffensesFormatter.instance_variable_set(:@offenses_formatters, new_formatters) - end + def reset_formatters + Packwerk::OffensesFormatter.instance_variable_set(:@formatter_by_identifier, nil) + current_formatters = Packwerk::OffensesFormatter.instance_variable_get(:@offenses_formatters) + new_formatters = current_formatters.delete_if { |f| f.new.identifier == "my_offenses_formatter" } + Packwerk::OffensesFormatter.instance_variable_set(:@offenses_formatters, new_formatters) + end - def teardown_application_fixture - Dir.chdir(@old_working_dir) - FileUtils.remove_entry(@app_dir, true) if using_template? - end + def teardown_application_fixture + Dir.chdir(@old_working_dir) + FileUtils.remove_entry(@app_dir, true) if using_template? + end - def use_template(template) - raise "use_template may only be called once per test" if using_template? + def use_template(template) + raise "use_template may only be called once per test" if using_template? - copy_dir("test/fixtures/#{template}") - Dir.chdir(app_dir) - end - - def app_dir - unless using_template? - raise "You need to set up an application fixture by calling `use_template(:the_template)`." + copy_dir("test/fixtures/#{template}") + Dir.chdir(app_dir) end - @app_dir - end + def app_dir + unless using_template? + raise "You need to set up an application fixture by calling `use_template(:the_template)`." + end - def config - @config ||= Packwerk::Configuration.from_path(app_dir) - end + @app_dir + end - def package_set - @package_set ||= Packwerk::PackageSet.load_all_from(config.root_path) - end + def config + @config ||= Configuration.from_path(app_dir) + end - def to_app_path(relative_path) - File.join(app_dir, relative_path) - end + def package_set + @package_set ||= Packwerk::PackageSet.load_all_from(config.root_path) + end - def to_app_paths(*relative_paths) - relative_paths.map { |path| to_app_path(path) } - end + def to_app_path(relative_path) + File.join(app_dir, relative_path) + end - def merge_into_app_yaml_file(relative_path, hash) - path = to_app_path(relative_path) - YamlFile.new(path).merge(hash) - end + def to_app_paths(*relative_paths) + relative_paths.map { |path| to_app_path(path) } + end - def remove_app_entry(relative_path) - FileUtils.remove_entry(to_app_path(relative_path)) - end + def merge_into_app_yaml_file(relative_path, hash) + path = to_app_path(relative_path) + YamlFile.new(path).merge(hash) + end - def open_app_file(path, mode: "w+", &block) - expanded_path = to_app_path(File.join(path)) - File.open(expanded_path, mode, &block) - end + def remove_app_entry(relative_path) + FileUtils.remove_entry(to_app_path(relative_path)) + end - # This gets cleaned up by `teardown_application_fixture` - def write_app_file(path, content) - expanded_path = to_app_path(File.join(*path)) - FileUtils.mkdir_p(Pathname.new(expanded_path).dirname) - File.open(expanded_path, "w+") do |file| - file.write(content) - file.flush + def open_app_file(path, mode: "w+", &block) + expanded_path = to_app_path(File.join(path)) + File.open(expanded_path, mode, &block) end - end - private + # This gets cleaned up by `teardown_application_fixture` + def write_app_file(path, content) + expanded_path = to_app_path(File.join(*path)) + FileUtils.mkdir_p(Pathname.new(expanded_path).dirname) + File.open(expanded_path, "w+") do |file| + file.write(content) + file.flush + end + end - def using_template? - defined? @app_dir - end + private - def copy_dir(path) - root = FileUtils.mkdir_p(fixture_path).last - FileUtils.cp_r("#{path}/.", T.must(root)) - @app_dir = root - end + def using_template? + defined? @app_dir + end + + def copy_dir(path) + root = FileUtils.mkdir_p(fixture_path).last + FileUtils.cp_r("#{path}/.", T.must(root)) + @app_dir = root + end - def fixture_path - T.bind(self, Minitest::Runnable) - File.join(TEMP_FIXTURE_DIR, "#{name}-#{Time.now.strftime("%Y_%m_%d_%H_%M_%S")}") + def fixture_path + T.bind(self, Minitest::Runnable) + File.join(TEMP_FIXTURE_DIR, "#{name}-#{Time.now.strftime("%Y_%m_%d_%H_%M_%S")}") + end end end diff --git a/test/support/factory_helper.rb b/test/support/factory_helper.rb index 6c0568e13..cf50332f8 100644 --- a/test/support/factory_helper.rb +++ b/test/support/factory_helper.rb @@ -1,24 +1,26 @@ # typed: true # frozen_string_literal: true -module FactoryHelper - def build_reference( - source_package: Packwerk::Package.new(name: "components/source", config: {}), - destination_package: Packwerk::Package.new(name: "components/destination", config: {}), - path: "some/path.rb", - constant_name: "::SomeName", - source_location: Packwerk::Node::Location.new(2, 12) - ) - constant = Packwerk::ConstantDiscovery::ConstantContext.new( - constant_name, - "some/location.rb", - destination_package, - ) - Packwerk::Reference.new( - package: source_package, - relative_path: path, - constant: constant, - source_location: source_location, +module Packwerk + module FactoryHelper + def build_reference( + source_package: Packwerk::Package.new(name: "components/source", config: {}), + destination_package: Packwerk::Package.new(name: "components/destination", config: {}), + path: "some/path.rb", + constant_name: "::SomeName", + source_location: Node::Location.new(2, 12) ) + constant = ConstantDiscovery::ConstantContext.new( + constant_name, + "some/location.rb", + destination_package, + ) + Packwerk::Reference.new( + package: source_package, + relative_path: path, + constant: constant, + source_location: source_location, + ) + end end end diff --git a/test/support/offenses_formatter_plain.rb b/test/support/offenses_formatter_plain.rb index 175ba427f..c0bfa915b 100644 --- a/test/support/offenses_formatter_plain.rb +++ b/test/support/offenses_formatter_plain.rb @@ -3,8 +3,8 @@ module Packwerk module Formatters - class OffensesFormatterPlain < OffensesFormatter - include Packwerk::OffensesFormatter + class OffensesFormatterPlain < DefaultOffensesFormatter + include OffensesFormatter IDENTIFIER = T.let("plain", String) extend T::Sig diff --git a/test/support/rails_application_fixture_helper.rb b/test/support/rails_application_fixture_helper.rb index 8f2914720..0d36bca52 100644 --- a/test/support/rails_application_fixture_helper.rb +++ b/test/support/rails_application_fixture_helper.rb @@ -5,7 +5,7 @@ require "zeitwerk" module RailsApplicationFixtureHelper - include ApplicationFixtureHelper + include Packwerk::ApplicationFixtureHelper class Autoloaders include Enumerable diff --git a/test/unit/application_validator_test.rb b/test/unit/application_validator_test.rb index 3878f5d8c..998d4bf9e 100644 --- a/test/unit/application_validator_test.rb +++ b/test/unit/application_validator_test.rb @@ -73,9 +73,9 @@ class ApplicationValidatorTest < Minitest::Test private - sig { returns(Packwerk::ApplicationValidator) } + sig { returns(ApplicationValidator) } def validator - @application_validator ||= Packwerk::ApplicationValidator.new + @application_validator ||= ApplicationValidator.new end end end diff --git a/test/unit/cache_test.rb b/test/unit/cache_test.rb index 79c168809..8cc4bbe85 100644 --- a/test/unit/cache_test.rb +++ b/test/unit/cache_test.rb @@ -43,7 +43,7 @@ class CacheTest < Minitest::Test configuration = Configuration.from_path configuration.stubs(cache_enabled?: true) - parse_run = Packwerk::ParseRun.new(relative_file_set: Set.new([filepath.to_s]), configuration: configuration) + parse_run = ParseRun.new(relative_file_set: Set.new([filepath.to_s]), configuration: configuration) parse_run.update_todo parse_run.update_todo diff --git a/test/unit/cli_test.rb b/test/unit/cli_test.rb index cea1e4c9e..d28e24082 100644 --- a/test/unit/cli_test.rb +++ b/test/unit/cli_test.rb @@ -96,7 +96,7 @@ class CliTest < Minitest::Test cli = ::Packwerk::Cli.new(out: string_io) validator = typed_mock(check_all: ApplicationValidator::Result.new(ok: true)) - Packwerk::ApplicationValidator.expects(:new).returns(validator) + ApplicationValidator.expects(:new).returns(validator) success = cli.execute_command(["validate"]) @@ -110,7 +110,7 @@ class CliTest < Minitest::Test cli = ::Packwerk::Cli.new(out: string_io) validator = typed_mock(check_all: ApplicationValidator::Result.new(ok: false, error_value: "I'm an error")) - Packwerk::ApplicationValidator.expects(:new).returns(validator) + ApplicationValidator.expects(:new).returns(validator) success = cli.execute_command(["validate"]) diff --git a/test/unit/file_processor_test.rb b/test/unit/file_processor_test.rb index 570bef7f9..931a0bb7d 100644 --- a/test/unit/file_processor_test.rb +++ b/test/unit/file_processor_test.rb @@ -16,7 +16,7 @@ class FileProcessorTest < Minitest::Test cache_directory: Pathname.new("tmp/cache/packwerk"), config_path: "packwerk.yml" ) - @file_processor = ::Packwerk::FileProcessor.new(node_processor_factory: @node_processor_factory, cache: @cache) + @file_processor = FileProcessor.new(node_processor_factory: @node_processor_factory, cache: @cache) end test "#call visits all nodes in a file path with no references" do @@ -100,7 +100,7 @@ class FileProcessorTest < Minitest::Test test "#call with an invalid syntax doesn't parse node" do @node_processor_factory.expects(:for).never - file_processor = ::Packwerk::FileProcessor.new( + file_processor = FileProcessor.new( node_processor_factory: @node_processor_factory, cache: Cache.new( enable_cache: false, diff --git a/test/unit/files_for_processing_test.rb b/test/unit/files_for_processing_test.rb index f21f4df48..8972d4ef7 100644 --- a/test/unit/files_for_processing_test.rb +++ b/test/unit/files_for_processing_test.rb @@ -11,7 +11,7 @@ class FilesForProcessingTest < Minitest::Test setup_application_fixture use_template(:skeleton) @package_path = "components/sales" - @configuration = ::Packwerk::Configuration.from_path + @configuration = Configuration.from_path end teardown do @@ -19,7 +19,7 @@ class FilesForProcessingTest < Minitest::Test end test "fetch with custom paths includes only include glob in custom paths" do - files = ::Packwerk::FilesForProcessing.fetch( + files = FilesForProcessing.fetch( relative_file_paths: [@package_path], configuration: @configuration, ).files @@ -28,7 +28,7 @@ class FilesForProcessingTest < Minitest::Test end test "fetch with custom paths excludes the exclude glob in custom paths" do - files = ::Packwerk::FilesForProcessing.fetch( + files = FilesForProcessing.fetch( relative_file_paths: [@package_path], configuration: @configuration ).files @@ -38,25 +38,25 @@ class FilesForProcessingTest < Minitest::Test end test "fetch with no custom paths includes only include glob across codebase" do - files = ::Packwerk::FilesForProcessing.fetch(relative_file_paths: [], configuration: @configuration).files + files = FilesForProcessing.fetch(relative_file_paths: [], configuration: @configuration).files assert_all_match(files, @configuration.include) end test "fetch with no custom paths excludes the exclude glob across codebase" do - files = ::Packwerk::FilesForProcessing.fetch(relative_file_paths: [], configuration: @configuration).files + files = FilesForProcessing.fetch(relative_file_paths: [], configuration: @configuration).files excluded_file_patterns = @configuration.exclude.map { |pattern| File.join(@configuration.root_path, pattern) } refute_any_match(files, Set.new(excluded_file_patterns)) end test "fetch does not return duplicated file paths" do - files = ::Packwerk::FilesForProcessing.fetch(relative_file_paths: [], configuration: @configuration).files + files = FilesForProcessing.fetch(relative_file_paths: [], configuration: @configuration).files assert_equal files, Set.new(files) end test "fetch with custom paths without ignoring nested packages includes only include glob in custom paths including nested package files" do - files = ::Packwerk::FilesForProcessing.fetch( + files = FilesForProcessing.fetch( relative_file_paths: ["."], configuration: @configuration, ignore_nested_packages: false @@ -66,7 +66,7 @@ class FilesForProcessingTest < Minitest::Test end test "fetch with no custom paths ignoring nested packages includes only include glob across codebase" do - files = ::Packwerk::FilesForProcessing.fetch( + files = FilesForProcessing.fetch( relative_file_paths: [], configuration: @configuration, ignore_nested_packages: true @@ -76,7 +76,7 @@ class FilesForProcessingTest < Minitest::Test end test "fetch with custom paths ignoring nested packages includes only include glob in custom paths without nested package files" do - files = ::Packwerk::FilesForProcessing.fetch( + files = FilesForProcessing.fetch( relative_file_paths: ["."], configuration: @configuration, ignore_nested_packages: true @@ -87,7 +87,7 @@ class FilesForProcessingTest < Minitest::Test end test "fetch with custom paths for files includes only include glob in custom paths" do - files = ::Packwerk::FilesForProcessing.fetch( + files = FilesForProcessing.fetch( relative_file_paths: [ "components/sales/app/models/order.rb", "components/sales/app/views/order.html.erb", diff --git a/test/unit/generators/configuration_file_test.rb b/test/unit/generators/configuration_file_test.rb index 31dd10afe..470a906eb 100644 --- a/test/unit/generators/configuration_file_test.rb +++ b/test/unit/generators/configuration_file_test.rb @@ -16,13 +16,13 @@ class ConfigurationFileTest < Minitest::Test end test ".generate creates a default configuration file if there were empty load paths array" do - generated_file_path = File.join(@temp_dir, Packwerk::Configuration::DEFAULT_CONFIG_PATH) + generated_file_path = File.join(@temp_dir, Configuration::DEFAULT_CONFIG_PATH) assert(Generators::ConfigurationFile.generate(root: @temp_dir, out: @string_io)) assert(File.exist?(generated_file_path)) end test ".generate does not create a configuration file if a file exists" do - file_path = File.join(@temp_dir, Packwerk::Configuration::DEFAULT_CONFIG_PATH) + file_path = File.join(@temp_dir, Configuration::DEFAULT_CONFIG_PATH) File.open(file_path, "w") do |_f| assert(Generators::ConfigurationFile.generate( root: @temp_dir, diff --git a/test/unit/node_visitor_test.rb b/test/unit/node_visitor_test.rb index ccad4e6ee..637fa8de5 100644 --- a/test/unit/node_visitor_test.rb +++ b/test/unit/node_visitor_test.rb @@ -11,7 +11,7 @@ class NodeVisitorTest < Minitest::Test test "#visit visits the correct number of nodes" do node_processor = typed_mock node_processor.expects(:call).times(3).returns(["an offense"]) - file_node_visitor = Packwerk::NodeVisitor.new(node_processor: node_processor) + file_node_visitor = NodeVisitor.new(node_processor: node_processor) node = ParserTestHelper.parse("class Hello; world; end") result = [] diff --git a/test/unit/parse_run_test.rb b/test/unit/parse_run_test.rb index eb967ff84..391498fc6 100644 --- a/test/unit/parse_run_test.rb +++ b/test/unit/parse_run_test.rb @@ -22,7 +22,7 @@ class ParseRunTest < Minitest::Test RunContext.any_instance.stubs(:process_file).returns([]) OffenseCollection.any_instance.expects(:dump_package_todo_files).once - parse_run = Packwerk::ParseRun.new( + parse_run = ParseRun.new( relative_file_set: Set.new(["path/of/exile.rb"]), configuration: Configuration.from_path ) @@ -41,7 +41,7 @@ class ParseRunTest < Minitest::Test RunContext.any_instance.stubs(:process_file).returns([offense]) OffenseCollection.any_instance.expects(:dump_package_todo_files).once - parse_run = Packwerk::ParseRun.new( + parse_run = ParseRun.new( relative_file_set: Set.new(["path/of/exile.rb"]), configuration: Configuration.from_path ) @@ -62,7 +62,7 @@ class ParseRunTest < Minitest::Test test "#update-todo returns exit code 1 when ran with file args" do use_template(:minimal) - parse_run = Packwerk::ParseRun.new( + parse_run = ParseRun.new( relative_file_set: Set.new(["path/of/exile.rb"]), file_set_specified: true, configuration: Configuration.from_path @@ -106,7 +106,7 @@ def something - a/b/c.rb YML - parse_run = Packwerk::ParseRun.new( + parse_run = ParseRun.new( relative_file_set: Set.new(["app/models/my_model.rb", "components/sales/app/models/order.rb"]), configuration: Configuration.from_path ) @@ -133,7 +133,7 @@ def something PackageTodo.any_instance.stubs(:listed?).returns(true) out = StringIO.new - parse_run = Packwerk::ParseRun.new( + parse_run = ParseRun.new( relative_file_set: Set.new(["some/path.rb"]), configuration: Configuration.new({ "parallel" => false }), progress_formatter: Formatters::ProgressFormatter.new(out) @@ -210,7 +210,7 @@ def something ) out = StringIO.new - parse_run = Packwerk::ParseRun.new( + parse_run = ParseRun.new( relative_file_set: Set.new([file_to_check]), configuration: Configuration.new({ "parallel" => false }), progress_formatter: Formatters::ProgressFormatter.new(out) @@ -256,7 +256,7 @@ def something YML out = StringIO.new - parse_run = Packwerk::ParseRun.new( + parse_run = ParseRun.new( relative_file_set: Set.new([file_to_check]), configuration: Configuration.new({ "parallel" => false }), progress_formatter: Formatters::ProgressFormatter.new(out) @@ -303,7 +303,7 @@ def something YML out = StringIO.new - parse_run = Packwerk::ParseRun.new( + parse_run = ParseRun.new( relative_file_set: Set.new([file_to_check]), configuration: Configuration.new({ "parallel" => false }), progress_formatter: Formatters::ProgressFormatter.new(out) @@ -350,7 +350,7 @@ def something YML out = StringIO.new - parse_run = Packwerk::ParseRun.new( + parse_run = ParseRun.new( relative_file_set: Set.new(["components/source/some/path.rb"]), configuration: Configuration.new({ "parallel" => false }), progress_formatter: Formatters::ProgressFormatter.new(out) @@ -406,7 +406,7 @@ def something out = StringIO.new - parse_run = Packwerk::ParseRun.new( + parse_run = ParseRun.new( relative_file_set: Set.new([file_to_check]), configuration: Configuration.new({ "parallel" => false, "offenses_formatter" => "plain" }), progress_formatter: Formatters::ProgressFormatter.new(out) @@ -454,7 +454,7 @@ def something PackageTodo.any_instance.stubs(:listed?).returns(true) OffenseCollection.any_instance.stubs(:stale_violations?).returns(true) out = StringIO.new - parse_run = Packwerk::ParseRun.new( + parse_run = ParseRun.new( relative_file_set: Set.new(["some/path.rb"]), configuration: Configuration.new({ "parallel" => false }), progress_formatter: Formatters::ProgressFormatter.new(out) @@ -491,7 +491,7 @@ def something message: "some message", violation_type: ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE ) - parse_run = Packwerk::ParseRun.new( + parse_run = ParseRun.new( relative_file_set: Set.new(["some/path.rb", "some/other_path.rb"]), configuration: Configuration.new ) diff --git a/test/unit/reference_extractor_test.rb b/test/unit/reference_extractor_test.rb index 028c83c4e..20bcbcff2 100644 --- a/test/unit/reference_extractor_test.rb +++ b/test/unit/reference_extractor_test.rb @@ -20,7 +20,7 @@ def setup resolver = ConstantResolver.new(root_path: app_dir, load_paths: load_paths) packages = ::Packwerk::PackageSet.load_all_from(app_dir) - @context_provider = ::Packwerk::ConstantDiscovery.new( + @context_provider = ConstantDiscovery.new( constant_resolver: resolver, packages: packages ) @@ -214,7 +214,7 @@ def teardown private class DummyAssociationInspector - include Packwerk::ConstantNameInspector + include ConstantNameInspector def initialize(association: false, reference_name: "Dummy", expected_args: nil) @association = association @@ -241,7 +241,7 @@ def process(code, file_path, constant_name_inspectors = DEFAULT_INSPECTORS) root_node = ParserTestHelper.parse(code) file_path = to_app_path(file_path) - extractor = ::Packwerk::ReferenceExtractor.new( + extractor = ReferenceExtractor.new( constant_name_inspectors: constant_name_inspectors, root_node: root_node, root_path: app_dir @@ -254,7 +254,7 @@ def process(code, file_path, constant_name_inspectors = DEFAULT_INSPECTORS) file_path: Pathname.new(file_path).relative_path_from(app_dir).to_s ) - ::Packwerk::ReferenceExtractor.get_fully_qualified_references_from( + ReferenceExtractor.get_fully_qualified_references_from( unresolved_references, @context_provider ) From 65d08ad60b8a5294df6ac3d3f060464d7e38359a Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 12 Dec 2022 09:14:45 -0500 Subject: [PATCH 06/16] Verify private_constant will not load code --- lib/packwerk.rb | 4 ++++ lib/packwerk/test_thing.rb | 9 +++++++++ 2 files changed, 13 insertions(+) create mode 100644 lib/packwerk/test_thing.rb diff --git a/lib/packwerk.rb b/lib/packwerk.rb index ed6530c75..b97080bd5 100644 --- a/lib/packwerk.rb +++ b/lib/packwerk.rb @@ -27,6 +27,10 @@ module Packwerk autoload :ReferenceOffense autoload :Result + autoload :TestThing + # Calling `private_constant` and running test does not fail the test suite + private_constant :TestThing + # Private APIs # Please submit an issue if you have a use-case for these autoload :ApplicationLoadPaths diff --git a/lib/packwerk/test_thing.rb b/lib/packwerk/test_thing.rb new file mode 100644 index 000000000..23d2bd696 --- /dev/null +++ b/lib/packwerk/test_thing.rb @@ -0,0 +1,9 @@ +# typed: strict +# frozen_string_literal: true + +module Packwerk + # This is a test module to see if `private_constant` will attempt to load the class + module TestThing + raise "Uh oh, we are loading!" + end +end From 5a7302d796ec9ce469ae61cbcfe8e164aa77c6f5 Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 12 Dec 2022 09:15:41 -0500 Subject: [PATCH 07/16] Verify autoload allows the constant to be registered without loading it --- lib/packwerk.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/packwerk.rb b/lib/packwerk.rb index b97080bd5..16ce9760f 100644 --- a/lib/packwerk.rb +++ b/lib/packwerk.rb @@ -27,7 +27,8 @@ module Packwerk autoload :ReferenceOffense autoload :Result - autoload :TestThing + # ./packwerk/lib/packwerk.rb:32:in `private_constant': constant Packwerk::TestThing not defined (NameError) + # autoload :TestThing # Calling `private_constant` and running test does not fail the test suite private_constant :TestThing From 879aea429b95ea2927cad4cf266bd80ef9b530d4 Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 12 Dec 2022 09:16:01 -0500 Subject: [PATCH 08/16] Verify loading TestThing breaks the test suite --- lib/packwerk.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/packwerk.rb b/lib/packwerk.rb index 16ce9760f..6f0550c11 100644 --- a/lib/packwerk.rb +++ b/lib/packwerk.rb @@ -27,10 +27,10 @@ module Packwerk autoload :ReferenceOffense autoload :Result - # ./packwerk/lib/packwerk.rb:32:in `private_constant': constant Packwerk::TestThing not defined (NameError) - # autoload :TestThing + autoload :TestThing # Calling `private_constant` and running test does not fail the test suite private_constant :TestThing + TestThing # Private APIs # Please submit an issue if you have a use-case for these From 0b584f624817a49c182bd7622c1f22e3bdf2731b Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 12 Dec 2022 09:16:24 -0500 Subject: [PATCH 09/16] Remove verification code --- lib/packwerk.rb | 5 ----- lib/packwerk/test_thing.rb | 9 --------- 2 files changed, 14 deletions(-) delete mode 100644 lib/packwerk/test_thing.rb diff --git a/lib/packwerk.rb b/lib/packwerk.rb index 6f0550c11..ed6530c75 100644 --- a/lib/packwerk.rb +++ b/lib/packwerk.rb @@ -27,11 +27,6 @@ module Packwerk autoload :ReferenceOffense autoload :Result - autoload :TestThing - # Calling `private_constant` and running test does not fail the test suite - private_constant :TestThing - TestThing - # Private APIs # Please submit an issue if you have a use-case for these autoload :ApplicationLoadPaths diff --git a/lib/packwerk/test_thing.rb b/lib/packwerk/test_thing.rb deleted file mode 100644 index 23d2bd696..000000000 --- a/lib/packwerk/test_thing.rb +++ /dev/null @@ -1,9 +0,0 @@ -# typed: strict -# frozen_string_literal: true - -module Packwerk - # This is a test module to see if `private_constant` will attempt to load the class - module TestThing - raise "Uh oh, we are loading!" - end -end From 64865d6e5f6f883d6d51568ab5480a732cd033ff Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 12 Dec 2022 09:19:55 -0500 Subject: [PATCH 10/16] Result => Cli::Result --- lib/packwerk.rb | 7 ++++++- lib/packwerk/cli/result.rb | 11 +++++++++++ lib/packwerk/parse_run.rb | 10 +++++----- lib/packwerk/result.rb | 9 --------- 4 files changed, 22 insertions(+), 15 deletions(-) create mode 100644 lib/packwerk/cli/result.rb delete mode 100644 lib/packwerk/result.rb diff --git a/lib/packwerk.rb b/lib/packwerk.rb index ed6530c75..7ccb8e50d 100644 --- a/lib/packwerk.rb +++ b/lib/packwerk.rb @@ -25,7 +25,6 @@ module Packwerk autoload :Parsers autoload :Reference autoload :ReferenceOffense - autoload :Result # Private APIs # Please submit an issue if you have a use-case for these @@ -82,6 +81,12 @@ module Packwerk autoload :Version private_constant :Version + class Cli + extend ActiveSupport::Autoload + + autoload :Result + end + module OutputStyles extend ActiveSupport::Autoload diff --git a/lib/packwerk/cli/result.rb b/lib/packwerk/cli/result.rb new file mode 100644 index 000000000..b106820d9 --- /dev/null +++ b/lib/packwerk/cli/result.rb @@ -0,0 +1,11 @@ +# typed: strict +# frozen_string_literal: true + +module Packwerk + class Cli + class Result < T::Struct + const :message, String + const :status, T::Boolean + end + end +end diff --git a/lib/packwerk/parse_run.rb b/lib/packwerk/parse_run.rb index f07d8d288..b4f209133 100644 --- a/lib/packwerk/parse_run.rb +++ b/lib/packwerk/parse_run.rb @@ -35,14 +35,14 @@ def initialize( @file_set_specified = file_set_specified end - sig { returns(Result) } + sig { returns(Cli::Result) } def update_todo if @file_set_specified message = <<~MSG.squish ⚠️ update-todo must be called without any file arguments. MSG - return Result.new(message: message, status: false) + return Cli::Result.new(message: message, status: false) end run_context = RunContext.from_configuration(@configuration) @@ -54,10 +54,10 @@ def update_todo ✅ `package_todo.yml` has been updated. EOS - Result.new(message: message, status: offense_collection.errors.empty?) + Cli::Result.new(message: message, status: offense_collection.errors.empty?) end - sig { returns(Result) } + sig { returns(Cli::Result) } def check run_context = RunContext.from_configuration(@configuration) offense_collection = find_offenses(run_context, show_errors: true) @@ -71,7 +71,7 @@ def check result_status = offense_collection.outstanding_offenses.empty? && !offense_collection.stale_violations?(@relative_file_set) && offense_collection.strict_mode_violations.empty? - Result.new(message: messages.select(&:present?).join("\n") + "\n", status: result_status) + Cli::Result.new(message: messages.select(&:present?).join("\n") + "\n", status: result_status) end private diff --git a/lib/packwerk/result.rb b/lib/packwerk/result.rb deleted file mode 100644 index 48f1ca2a7..000000000 --- a/lib/packwerk/result.rb +++ /dev/null @@ -1,9 +0,0 @@ -# typed: strict -# frozen_string_literal: true - -module Packwerk - class Result < T::Struct - const :message, String - const :status, T::Boolean - end -end From fd951be32ba40e39adb6dc2add6e165f82dab43d Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 12 Dec 2022 09:25:44 -0500 Subject: [PATCH 11/16] ApplicationValidator::Result => Validator::Result --- USAGE.md | 2 +- lib/packwerk.rb | 10 +++++-- lib/packwerk/application_validator.rb | 28 +++++++++---------- lib/packwerk/cli.rb | 2 +- lib/packwerk/validator.rb | 10 +++---- .../result.rb | 2 +- .../validators/dependency_validator.rb | 20 ++++++------- test/unit/cli_test.rb | 4 +-- 8 files changed, 41 insertions(+), 37 deletions(-) rename lib/packwerk/{application_validator => validator}/result.rb (90%) diff --git a/USAGE.md b/USAGE.md index 688c68433..f59bf31c2 100644 --- a/USAGE.md +++ b/USAGE.md @@ -358,7 +358,7 @@ class MyValidator ['enforce_my_custom_checker'] end - sig { override.params(package_set: PackageSet, configuration: Configuration).returns(ApplicationValidator::Result) } + sig { override.params(package_set: PackageSet, configuration: Configuration).returns(Validator::Result) } def call(package_set, configuration) # your logic here end diff --git a/lib/packwerk.rb b/lib/packwerk.rb index 7ccb8e50d..61f4d5f0d 100644 --- a/lib/packwerk.rb +++ b/lib/packwerk.rb @@ -25,6 +25,7 @@ module Packwerk autoload :Parsers autoload :Reference autoload :ReferenceOffense + autoload :Validator # Private APIs # Please submit an issue if you have a use-case for these @@ -76,11 +77,15 @@ module Packwerk private_constant :RunContext autoload :UnresolvedReference private_constant :UnresolvedReference - autoload :Validator - private_constant :Validator autoload :Version private_constant :Version + module Validator + extend ActiveSupport::Autoload + + autoload :Result + end + class Cli extend ActiveSupport::Autoload @@ -133,7 +138,6 @@ module Checkers class ApplicationValidator extend ActiveSupport::Autoload - autoload :Result autoload :Helpers end end diff --git a/lib/packwerk/application_validator.rb b/lib/packwerk/application_validator.rb index e92b2bd55..563a59712 100644 --- a/lib/packwerk/application_validator.rb +++ b/lib/packwerk/application_validator.rb @@ -12,13 +12,13 @@ class ApplicationValidator include Validator extend T::Sig - sig { params(package_set: PackageSet, configuration: Configuration).returns(ApplicationValidator::Result) } + sig { params(package_set: PackageSet, configuration: Configuration).returns(Validator::Result) } def check_all(package_set, configuration) results = Validator.all.flat_map { |validator| validator.call(package_set, configuration) } merge_results(results) end - sig { override.params(package_set: PackageSet, configuration: Configuration).returns(ApplicationValidator::Result) } + sig { override.params(package_set: PackageSet, configuration: Configuration).returns(Validator::Result) } def call(package_set, configuration) results = [ check_package_manifest_syntax(configuration), @@ -37,7 +37,7 @@ def permitted_keys ] end - sig { params(configuration: Configuration).returns(Result) } + sig { params(configuration: Configuration).returns(Validator::Result) } def check_package_manifest_syntax(configuration) errors = [] @@ -54,10 +54,10 @@ def check_package_manifest_syntax(configuration) end if errors.empty? - Result.new(ok: true) + Validator::Result.new(ok: true) else merge_results( - errors.map { |error| Result.new(ok: false, error_value: error) }, + errors.map { |error| Validator::Result.new(ok: false, error_value: error) }, separator: "\n", before_errors: "Malformed syntax in the following manifests:\n\n", after_errors: "\n", @@ -65,7 +65,7 @@ def check_package_manifest_syntax(configuration) end end - sig { params(configuration: Configuration).returns(Result) } + sig { params(configuration: Configuration).returns(Validator::Result) } def check_application_structure(configuration) resolver = ConstantResolver.new( root_path: configuration.root_path.to_s, @@ -74,13 +74,13 @@ def check_application_structure(configuration) begin resolver.file_map - Result.new(ok: true) + Validator::Result.new(ok: true) rescue => e - Result.new(ok: false, error_value: e.message) + Validator::Result.new(ok: false, error_value: e.message) end end - sig { params(configuration: Configuration).returns(Result) } + sig { params(configuration: Configuration).returns(Validator::Result) } def check_package_manifest_paths(configuration) all_package_manifests = package_manifests(configuration, "**/") package_paths_package_manifests = package_manifests(configuration, package_glob(configuration)) @@ -88,9 +88,9 @@ def check_package_manifest_paths(configuration) difference = all_package_manifests - package_paths_package_manifests if difference.empty? - Result.new(ok: true) + Validator::Result.new(ok: true) else - Result.new( + Validator::Result.new( ok: false, error_value: <<~EOS Expected package paths for all package.ymls to be specified, but paths were missing for the following manifests: @@ -101,15 +101,15 @@ def check_package_manifest_paths(configuration) end end - sig { params(configuration: Configuration).returns(Result) } + sig { params(configuration: Configuration).returns(Validator::Result) } def check_root_package_exists(configuration) root_package_path = File.join(configuration.root_path, "package.yml") all_packages_manifests = package_manifests(configuration, package_glob(configuration)) if all_packages_manifests.include?(root_package_path) - Result.new(ok: true) + Validator::Result.new(ok: true) else - Result.new( + Validator::Result.new( ok: false, error_value: <<~EOS A root package does not exist. Create an empty `package.yml` at the root directory. diff --git a/lib/packwerk/cli.rb b/lib/packwerk/cli.rb index 0d86c73cc..19efbeea4 100644 --- a/lib/packwerk/cli.rb +++ b/lib/packwerk/cli.rb @@ -170,7 +170,7 @@ def package_set ) end - sig { params(result: ApplicationValidator::Result).void } + sig { params(result: Validator::Result).void } def list_validation_errors(result) @out.puts if result.ok? diff --git a/lib/packwerk/validator.rb b/lib/packwerk/validator.rb index 307295196..02f07e728 100644 --- a/lib/packwerk/validator.rb +++ b/lib/packwerk/validator.rb @@ -32,7 +32,7 @@ def all def permitted_keys end - sig { abstract.params(package_set: PackageSet, configuration: Configuration).returns(ApplicationValidator::Result) } + sig { abstract.params(package_set: PackageSet, configuration: Configuration).returns(Validator::Result) } def call(package_set, configuration) end @@ -58,19 +58,19 @@ def package_glob(configuration) sig do params( - results: T::Array[ApplicationValidator::Result], + results: T::Array[Validator::Result], separator: String, before_errors: String, after_errors: String, - ).returns(ApplicationValidator::Result) + ).returns(Validator::Result) end def merge_results(results, separator: "\n", before_errors: "", after_errors: "") results.reject!(&:ok?) if results.empty? - ApplicationValidator::Result.new(ok: true) + Validator::Result.new(ok: true) else - ApplicationValidator::Result.new( + Validator::Result.new( ok: false, error_value: [ before_errors, diff --git a/lib/packwerk/application_validator/result.rb b/lib/packwerk/validator/result.rb similarity index 90% rename from lib/packwerk/application_validator/result.rb rename to lib/packwerk/validator/result.rb index d75cb0cb0..96d901476 100644 --- a/lib/packwerk/application_validator/result.rb +++ b/lib/packwerk/validator/result.rb @@ -2,7 +2,7 @@ # frozen_string_literal: true module Packwerk - class ApplicationValidator + module Validator class Result < T::Struct extend T::Sig diff --git a/lib/packwerk/validators/dependency_validator.rb b/lib/packwerk/validators/dependency_validator.rb index 9313e7fc8..7a8137292 100644 --- a/lib/packwerk/validators/dependency_validator.rb +++ b/lib/packwerk/validators/dependency_validator.rb @@ -8,7 +8,7 @@ class DependencyValidator include Validator sig do - override.params(package_set: PackageSet, configuration: Configuration).returns(ApplicationValidator::Result) + override.params(package_set: PackageSet, configuration: Configuration).returns(Validator::Result) end def call(package_set, configuration) results = [ @@ -30,7 +30,7 @@ def permitted_keys private - sig { params(configuration: Configuration).returns(ApplicationValidator::Result) } + sig { params(configuration: Configuration).returns(Validator::Result) } def check_package_manifest_syntax(configuration) errors = [] @@ -51,10 +51,10 @@ def check_package_manifest_syntax(configuration) end if errors.empty? - ApplicationValidator::Result.new(ok: true) + Validator::Result.new(ok: true) else merge_results( - errors.map { |error| ApplicationValidator::Result.new(ok: false, error_value: error) }, + errors.map { |error| Validator::Result.new(ok: false, error_value: error) }, separator: "\n", before_errors: "Malformed syntax in the following manifests:\n\n", after_errors: "\n", @@ -62,7 +62,7 @@ def check_package_manifest_syntax(configuration) end end - sig { params(package_set: PackageSet).returns(ApplicationValidator::Result) } + sig { params(package_set: PackageSet).returns(Validator::Result) } def check_acyclic_graph(package_set) edges = package_set.flat_map do |package| package.dependencies.map do |dependency| @@ -75,9 +75,9 @@ def check_acyclic_graph(package_set) cycle_strings = build_cycle_strings(dependency_graph.cycles) if dependency_graph.acyclic? - ApplicationValidator::Result.new(ok: true) + Validator::Result.new(ok: true) else - ApplicationValidator::Result.new( + Validator::Result.new( ok: false, error_value: <<~EOS Expected the package dependency graph to be acyclic, but it contains the following circular dependencies: @@ -88,7 +88,7 @@ def check_acyclic_graph(package_set) end end - sig { params(configuration: Configuration).returns(ApplicationValidator::Result) } + sig { params(configuration: Configuration).returns(Validator::Result) } def check_valid_package_dependencies(configuration) packages_dependencies = package_manifests_settings_for(configuration, "dependencies") .delete_if { |_, deps| deps.nil? } @@ -104,7 +104,7 @@ def check_valid_package_dependencies(configuration) end if packages_with_invalid_dependencies.empty? - ApplicationValidator::Result.new(ok: true) + Validator::Result.new(ok: true) else error_locations = packages_with_invalid_dependencies.map do |package, invalid_dependencies| package ||= configuration.root_path @@ -117,7 +117,7 @@ def check_valid_package_dependencies(configuration) EOS end - ApplicationValidator::Result.new( + Validator::Result.new( ok: false, error_value: "These dependencies do not point to valid packages:\n\n#{error_locations.join("\n")}" ) diff --git a/test/unit/cli_test.rb b/test/unit/cli_test.rb index d28e24082..9fc35faf6 100644 --- a/test/unit/cli_test.rb +++ b/test/unit/cli_test.rb @@ -95,7 +95,7 @@ class CliTest < Minitest::Test string_io = StringIO.new cli = ::Packwerk::Cli.new(out: string_io) - validator = typed_mock(check_all: ApplicationValidator::Result.new(ok: true)) + validator = typed_mock(check_all: Validator::Result.new(ok: true)) ApplicationValidator.expects(:new).returns(validator) success = cli.execute_command(["validate"]) @@ -109,7 +109,7 @@ class CliTest < Minitest::Test string_io = StringIO.new cli = ::Packwerk::Cli.new(out: string_io) - validator = typed_mock(check_all: ApplicationValidator::Result.new(ok: false, error_value: "I'm an error")) + validator = typed_mock(check_all: Validator::Result.new(ok: false, error_value: "I'm an error")) ApplicationValidator.expects(:new).returns(validator) success = cli.execute_command(["validate"]) From 50c4ea6e09c00476dec5658259de62cf77d06bc6 Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 12 Dec 2022 09:30:16 -0500 Subject: [PATCH 12/16] Add Private module --- lib/packwerk.rb | 3 +++ lib/packwerk/private.rb | 7 +++++++ 2 files changed, 10 insertions(+) create mode 100644 lib/packwerk/private.rb diff --git a/lib/packwerk.rb b/lib/packwerk.rb index 61f4d5f0d..25007f384 100644 --- a/lib/packwerk.rb +++ b/lib/packwerk.rb @@ -80,6 +80,9 @@ module Packwerk autoload :Version private_constant :Version + autoload :Private + private_constant :Private + module Validator extend ActiveSupport::Autoload diff --git a/lib/packwerk/private.rb b/lib/packwerk/private.rb new file mode 100644 index 000000000..322608100 --- /dev/null +++ b/lib/packwerk/private.rb @@ -0,0 +1,7 @@ +# typed: strict +# frozen_string_literal: true + +module Packwerk + module Private + end +end From 8b041ba8b59916f5b99476151e58a34d1ac2f8cb Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 12 Dec 2022 09:35:01 -0500 Subject: [PATCH 13/16] Move autoloads into Private --- lib/packwerk.rb | 91 ++--------------------------------------- lib/packwerk/private.rb | 64 +++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 88 deletions(-) diff --git a/lib/packwerk.rb b/lib/packwerk.rb index 25007f384..0e80d7e26 100644 --- a/lib/packwerk.rb +++ b/lib/packwerk.rb @@ -27,62 +27,6 @@ module Packwerk autoload :ReferenceOffense autoload :Validator - # Private APIs - # Please submit an issue if you have a use-case for these - autoload :ApplicationLoadPaths - private_constant :ApplicationLoadPaths - autoload :ApplicationValidator - private_constant :ApplicationValidator - autoload :AssociationInspector - private_constant :AssociationInspector - autoload :Cache - private_constant :Cache - autoload :Configuration - private_constant :Configuration - autoload :ConstantDiscovery - private_constant :ConstantDiscovery - autoload :ConstantNameInspector - private_constant :ConstantNameInspector - autoload :ConstNodeInspector - private_constant :ConstNodeInspector - autoload :ExtensionLoader - private_constant :ExtensionLoader - autoload :FileProcessor - private_constant :FileProcessor - autoload :FilesForProcessing - private_constant :FilesForProcessing - autoload :Graph - private_constant :Graph - autoload :Loader - private_constant :Loader - autoload :Node - private_constant :Node - autoload :NodeHelpers - private_constant :NodeHelpers - autoload :NodeProcessor - private_constant :NodeProcessor - autoload :NodeProcessorFactory - private_constant :NodeProcessorFactory - autoload :NodeVisitor - private_constant :NodeVisitor - autoload :OffenseCollection - private_constant :OffenseCollection - autoload :ParsedConstantDefinitions - private_constant :ParsedConstantDefinitions - autoload :ParseRun - private_constant :ParseRun - autoload :ReferenceExtractor - private_constant :ReferenceExtractor - autoload :RunContext - private_constant :RunContext - autoload :UnresolvedReference - private_constant :UnresolvedReference - autoload :Version - private_constant :Version - - autoload :Private - private_constant :Private - module Validator extend ActiveSupport::Autoload @@ -106,43 +50,14 @@ module OutputStyles autoload :OffenseProgressMarker end - module Formatters - extend ActiveSupport::Autoload - - autoload :ProgressFormatter - end - - private_constant :Formatters - - module Generators - extend ActiveSupport::Autoload - - autoload :ConfigurationFile - autoload :RootPackage - end - - private_constant :Generators - - module ReferenceChecking - extend ActiveSupport::Autoload - - autoload :ReferenceChecker - - module Checkers - extend ActiveSupport::Autoload - - autoload :DependencyChecker - autoload :PrivacyChecker - end - end - - private_constant :ReferenceChecking - class ApplicationValidator extend ActiveSupport::Autoload autoload :Helpers end + + # Private APIs + autoload :Private end # Required to register the DefaultOffensesFormatter diff --git a/lib/packwerk/private.rb b/lib/packwerk/private.rb index 322608100..43a362d34 100644 --- a/lib/packwerk/private.rb +++ b/lib/packwerk/private.rb @@ -2,6 +2,70 @@ # frozen_string_literal: true module Packwerk + # Private APIs + # Please submit an issue if you have a use-case for these + module Private + extend ActiveSupport::Autoload + + autoload :ApplicationLoadPaths + autoload :ApplicationValidator + autoload :AssociationInspector + autoload :Cache + autoload :Configuration + autoload :ConstantDiscovery + autoload :ConstantNameInspector + autoload :ConstNodeInspector + autoload :ExtensionLoader + autoload :FileProcessor + autoload :FilesForProcessing + autoload :Graph + autoload :Loader + autoload :Node + autoload :NodeHelpers + autoload :NodeProcessor + autoload :NodeProcessorFactory + autoload :NodeVisitor + autoload :OffenseCollection + autoload :ParsedConstantDefinitions + autoload :ParseRun + autoload :ReferenceExtractor + autoload :RunContext + autoload :UnresolvedReference + autoload :Version + + module Formatters + extend ActiveSupport::Autoload + + autoload :ProgressFormatter + end + + private_constant :Formatters + + module Generators + extend ActiveSupport::Autoload + + autoload :ConfigurationFile + autoload :RootPackage + end + + private_constant :Generators + + module ReferenceChecking + extend ActiveSupport::Autoload + + autoload :ReferenceChecker + + module Checkers + extend ActiveSupport::Autoload + + autoload :DependencyChecker + autoload :PrivacyChecker + end + end + + private_constant :ReferenceChecking end + + private_constant :Private end From 1eb2899f9a2cd20f045eb1fea93fc8eb7b07facd Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 12 Dec 2022 09:37:18 -0500 Subject: [PATCH 14/16] move files into private --- lib/packwerk.rb | 1 + lib/packwerk/private.rb | 2 -- lib/packwerk/{ => private}/application_load_paths.rb | 0 lib/packwerk/{ => private}/application_validator.rb | 0 lib/packwerk/{ => private}/association_inspector.rb | 0 lib/packwerk/{ => private}/cache.rb | 0 lib/packwerk/{ => private}/configuration.rb | 0 lib/packwerk/{ => private}/const_node_inspector.rb | 0 lib/packwerk/{ => private}/constant_discovery.rb | 0 lib/packwerk/{ => private}/constant_name_inspector.rb | 0 lib/packwerk/{ => private}/extension_loader.rb | 0 lib/packwerk/{ => private}/file_processor.rb | 0 lib/packwerk/{ => private}/files_for_processing.rb | 0 lib/packwerk/{ => private}/node.rb | 0 lib/packwerk/{ => private}/node_helpers.rb | 0 lib/packwerk/{ => private}/node_processor.rb | 0 lib/packwerk/{ => private}/node_processor_factory.rb | 0 lib/packwerk/{ => private}/node_visitor.rb | 0 lib/packwerk/{ => private}/offense_collection.rb | 0 lib/packwerk/{ => private}/parse_run.rb | 0 lib/packwerk/{ => private}/parsed_constant_definitions.rb | 0 lib/packwerk/{ => private}/reference_extractor.rb | 0 lib/packwerk/{ => private}/run_context.rb | 0 lib/packwerk/{ => private}/unresolved_reference.rb | 0 24 files changed, 1 insertion(+), 2 deletions(-) rename lib/packwerk/{ => private}/application_load_paths.rb (100%) rename lib/packwerk/{ => private}/application_validator.rb (100%) rename lib/packwerk/{ => private}/association_inspector.rb (100%) rename lib/packwerk/{ => private}/cache.rb (100%) rename lib/packwerk/{ => private}/configuration.rb (100%) rename lib/packwerk/{ => private}/const_node_inspector.rb (100%) rename lib/packwerk/{ => private}/constant_discovery.rb (100%) rename lib/packwerk/{ => private}/constant_name_inspector.rb (100%) rename lib/packwerk/{ => private}/extension_loader.rb (100%) rename lib/packwerk/{ => private}/file_processor.rb (100%) rename lib/packwerk/{ => private}/files_for_processing.rb (100%) rename lib/packwerk/{ => private}/node.rb (100%) rename lib/packwerk/{ => private}/node_helpers.rb (100%) rename lib/packwerk/{ => private}/node_processor.rb (100%) rename lib/packwerk/{ => private}/node_processor_factory.rb (100%) rename lib/packwerk/{ => private}/node_visitor.rb (100%) rename lib/packwerk/{ => private}/offense_collection.rb (100%) rename lib/packwerk/{ => private}/parse_run.rb (100%) rename lib/packwerk/{ => private}/parsed_constant_definitions.rb (100%) rename lib/packwerk/{ => private}/reference_extractor.rb (100%) rename lib/packwerk/{ => private}/run_context.rb (100%) rename lib/packwerk/{ => private}/unresolved_reference.rb (100%) diff --git a/lib/packwerk.rb b/lib/packwerk.rb index 0e80d7e26..eb39c6508 100644 --- a/lib/packwerk.rb +++ b/lib/packwerk.rb @@ -26,6 +26,7 @@ module Packwerk autoload :Reference autoload :ReferenceOffense autoload :Validator + autoload :Version module Validator extend ActiveSupport::Autoload diff --git a/lib/packwerk/private.rb b/lib/packwerk/private.rb index 43a362d34..efc9626fd 100644 --- a/lib/packwerk/private.rb +++ b/lib/packwerk/private.rb @@ -20,7 +20,6 @@ module Private autoload :FileProcessor autoload :FilesForProcessing autoload :Graph - autoload :Loader autoload :Node autoload :NodeHelpers autoload :NodeProcessor @@ -32,7 +31,6 @@ module Private autoload :ReferenceExtractor autoload :RunContext autoload :UnresolvedReference - autoload :Version module Formatters extend ActiveSupport::Autoload diff --git a/lib/packwerk/application_load_paths.rb b/lib/packwerk/private/application_load_paths.rb similarity index 100% rename from lib/packwerk/application_load_paths.rb rename to lib/packwerk/private/application_load_paths.rb diff --git a/lib/packwerk/application_validator.rb b/lib/packwerk/private/application_validator.rb similarity index 100% rename from lib/packwerk/application_validator.rb rename to lib/packwerk/private/application_validator.rb diff --git a/lib/packwerk/association_inspector.rb b/lib/packwerk/private/association_inspector.rb similarity index 100% rename from lib/packwerk/association_inspector.rb rename to lib/packwerk/private/association_inspector.rb diff --git a/lib/packwerk/cache.rb b/lib/packwerk/private/cache.rb similarity index 100% rename from lib/packwerk/cache.rb rename to lib/packwerk/private/cache.rb diff --git a/lib/packwerk/configuration.rb b/lib/packwerk/private/configuration.rb similarity index 100% rename from lib/packwerk/configuration.rb rename to lib/packwerk/private/configuration.rb diff --git a/lib/packwerk/const_node_inspector.rb b/lib/packwerk/private/const_node_inspector.rb similarity index 100% rename from lib/packwerk/const_node_inspector.rb rename to lib/packwerk/private/const_node_inspector.rb diff --git a/lib/packwerk/constant_discovery.rb b/lib/packwerk/private/constant_discovery.rb similarity index 100% rename from lib/packwerk/constant_discovery.rb rename to lib/packwerk/private/constant_discovery.rb diff --git a/lib/packwerk/constant_name_inspector.rb b/lib/packwerk/private/constant_name_inspector.rb similarity index 100% rename from lib/packwerk/constant_name_inspector.rb rename to lib/packwerk/private/constant_name_inspector.rb diff --git a/lib/packwerk/extension_loader.rb b/lib/packwerk/private/extension_loader.rb similarity index 100% rename from lib/packwerk/extension_loader.rb rename to lib/packwerk/private/extension_loader.rb diff --git a/lib/packwerk/file_processor.rb b/lib/packwerk/private/file_processor.rb similarity index 100% rename from lib/packwerk/file_processor.rb rename to lib/packwerk/private/file_processor.rb diff --git a/lib/packwerk/files_for_processing.rb b/lib/packwerk/private/files_for_processing.rb similarity index 100% rename from lib/packwerk/files_for_processing.rb rename to lib/packwerk/private/files_for_processing.rb diff --git a/lib/packwerk/node.rb b/lib/packwerk/private/node.rb similarity index 100% rename from lib/packwerk/node.rb rename to lib/packwerk/private/node.rb diff --git a/lib/packwerk/node_helpers.rb b/lib/packwerk/private/node_helpers.rb similarity index 100% rename from lib/packwerk/node_helpers.rb rename to lib/packwerk/private/node_helpers.rb diff --git a/lib/packwerk/node_processor.rb b/lib/packwerk/private/node_processor.rb similarity index 100% rename from lib/packwerk/node_processor.rb rename to lib/packwerk/private/node_processor.rb diff --git a/lib/packwerk/node_processor_factory.rb b/lib/packwerk/private/node_processor_factory.rb similarity index 100% rename from lib/packwerk/node_processor_factory.rb rename to lib/packwerk/private/node_processor_factory.rb diff --git a/lib/packwerk/node_visitor.rb b/lib/packwerk/private/node_visitor.rb similarity index 100% rename from lib/packwerk/node_visitor.rb rename to lib/packwerk/private/node_visitor.rb diff --git a/lib/packwerk/offense_collection.rb b/lib/packwerk/private/offense_collection.rb similarity index 100% rename from lib/packwerk/offense_collection.rb rename to lib/packwerk/private/offense_collection.rb diff --git a/lib/packwerk/parse_run.rb b/lib/packwerk/private/parse_run.rb similarity index 100% rename from lib/packwerk/parse_run.rb rename to lib/packwerk/private/parse_run.rb diff --git a/lib/packwerk/parsed_constant_definitions.rb b/lib/packwerk/private/parsed_constant_definitions.rb similarity index 100% rename from lib/packwerk/parsed_constant_definitions.rb rename to lib/packwerk/private/parsed_constant_definitions.rb diff --git a/lib/packwerk/reference_extractor.rb b/lib/packwerk/private/reference_extractor.rb similarity index 100% rename from lib/packwerk/reference_extractor.rb rename to lib/packwerk/private/reference_extractor.rb diff --git a/lib/packwerk/run_context.rb b/lib/packwerk/private/run_context.rb similarity index 100% rename from lib/packwerk/run_context.rb rename to lib/packwerk/private/run_context.rb diff --git a/lib/packwerk/unresolved_reference.rb b/lib/packwerk/private/unresolved_reference.rb similarity index 100% rename from lib/packwerk/unresolved_reference.rb rename to lib/packwerk/private/unresolved_reference.rb From cce2ad42f4ba01b34910d061a237fe59d4738235 Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 12 Dec 2022 09:55:08 -0500 Subject: [PATCH 15/16] Fix references --- lib/packwerk.rb | 26 +- lib/packwerk/cli.rb | 12 +- lib/packwerk/{private => }/configuration.rb | 4 +- lib/packwerk/offense.rb | 4 +- .../{private => }/offense_collection.rb | 0 lib/packwerk/private.rb | 33 +- .../private/application_load_paths.rb | 102 ++-- lib/packwerk/private/application_validator.rb | 200 +++---- lib/packwerk/private/association_inspector.rb | 106 ++-- lib/packwerk/private/cache.rb | 258 ++++----- lib/packwerk/private/const_node_inspector.rb | 86 +-- lib/packwerk/private/constant_discovery.rb | 124 ++-- .../private/constant_name_inspector.rb | 22 +- lib/packwerk/private/extension_loader.rb | 30 +- lib/packwerk/private/file_processor.rb | 124 ++-- lib/packwerk/private/files_for_processing.rb | 162 +++--- lib/packwerk/private/node.rb | 6 +- lib/packwerk/private/node_helpers.rb | 540 +++++++++--------- lib/packwerk/private/node_processor.rb | 46 +- .../private/node_processor_factory.rb | 42 +- lib/packwerk/private/node_visitor.rb | 28 +- lib/packwerk/private/parse_run.rb | 194 +++---- .../private/parsed_constant_definitions.rb | 80 +-- lib/packwerk/private/reference_extractor.rb | 196 +++---- lib/packwerk/private/run_context.rb | 222 +++---- lib/packwerk/private/unresolved_reference.rb | 24 +- lib/packwerk/reference_offense.rb | 2 +- .../packwerk/private/unresolved_reference.rbi | 35 ++ sorbet/rbi/shims/packwerk/reference.rbi | 8 +- .../shims/packwerk/unresolved_reference.rbi | 33 -- test/const_node_inspector_test.rb | 12 +- test/node_helpers_test.rb | 96 ++-- test/support/factory_helper.rb | 4 +- test/unit/application_load_paths_test.rb | 12 +- test/unit/application_validator_test.rb | 4 +- test/unit/association_inspector_test.rb | 16 +- test/unit/cache_test.rb | 12 +- test/unit/cli_test.rb | 28 +- test/unit/configuration_test.rb | 4 +- test/unit/constant_discovery_test.rb | 4 +- test/unit/file_processor_test.rb | 32 +- test/unit/files_for_processing_test.rb | 20 +- test/unit/node_visitor_test.rb | 2 +- test/unit/offense_test.rb | 2 +- test/unit/parse_run_test.rb | 46 +- test/unit/parsed_constant_definitions_test.rb | 36 +- test/unit/reference_extractor_test.rb | 18 +- 47 files changed, 1565 insertions(+), 1532 deletions(-) rename lib/packwerk/{private => }/configuration.rb (92%) rename lib/packwerk/{private => }/offense_collection.rb (100%) create mode 100644 sorbet/rbi/shims/packwerk/private/unresolved_reference.rbi delete mode 100644 sorbet/rbi/shims/packwerk/unresolved_reference.rbi diff --git a/lib/packwerk.rb b/lib/packwerk.rb index eb39c6508..bf6d9b232 100644 --- a/lib/packwerk.rb +++ b/lib/packwerk.rb @@ -16,7 +16,10 @@ module Packwerk # Public APIs autoload :Checker autoload :Cli + autoload :Configuration + autoload :Graph autoload :Offense + autoload :OffenseCollection autoload :OffensesFormatter autoload :OutputStyle autoload :Package @@ -28,6 +31,25 @@ module Packwerk autoload :Validator autoload :Version + module Generators + extend ActiveSupport::Autoload + + autoload :ConfigurationFile + autoload :RootPackage + end + + module ReferenceChecking + extend ActiveSupport::Autoload + + autoload :ReferenceChecker + + module Checkers + extend ActiveSupport::Autoload + + autoload :DependencyChecker + end + end + module Validator extend ActiveSupport::Autoload @@ -51,10 +73,10 @@ module OutputStyles autoload :OffenseProgressMarker end - class ApplicationValidator + module Formatters extend ActiveSupport::Autoload - autoload :Helpers + autoload :ProgressFormatter end # Private APIs diff --git a/lib/packwerk/cli.rb b/lib/packwerk/cli.rb index 19efbeea4..41a99bc04 100644 --- a/lib/packwerk/cli.rb +++ b/lib/packwerk/cli.rb @@ -130,10 +130,10 @@ def output_result(result) params( relative_file_paths: T::Array[String], ignore_nested_packages: T::Boolean - ).returns(FilesForProcessing) + ).returns(Private::FilesForProcessing) end def fetch_files_to_process(relative_file_paths, ignore_nested_packages) - files_for_processing = FilesForProcessing.fetch( + files_for_processing = Private::FilesForProcessing.fetch( relative_file_paths: relative_file_paths, ignore_nested_packages: ignore_nested_packages, configuration: @configuration @@ -157,9 +157,9 @@ def validate(_paths) end end - sig { returns(ApplicationValidator) } + sig { returns(Private::ApplicationValidator) } def validator - ApplicationValidator.new + Private::ApplicationValidator.new end sig { returns(PackageSet) } @@ -182,7 +182,7 @@ def list_validation_errors(result) end end - sig { params(args: T::Array[String]).returns(ParseRun) } + sig { params(args: T::Array[String]).returns(Private::ParseRun) } def parse_run(args) relative_file_paths = T.let([], T::Array[String]) ignore_nested_packages = nil @@ -211,7 +211,7 @@ def parse_run(args) files_for_processing = fetch_files_to_process(relative_file_paths, ignore_nested_packages) - ParseRun.new( + Private::ParseRun.new( relative_file_set: files_for_processing.files, file_set_specified: files_for_processing.files_specified?, configuration: @configuration, diff --git a/lib/packwerk/private/configuration.rb b/lib/packwerk/configuration.rb similarity index 92% rename from lib/packwerk/private/configuration.rb rename to lib/packwerk/configuration.rb index 9b96ed453..b46229dc9 100644 --- a/lib/packwerk/private/configuration.rb +++ b/lib/packwerk/configuration.rb @@ -55,13 +55,13 @@ def initialize(configs = {}, config_path: nil) if configs.key?("require") configs["require"].each do |require_directive| - ExtensionLoader.load(require_directive, @root_path) + Private::ExtensionLoader.load(require_directive, @root_path) end end end def load_paths - @load_paths ||= ApplicationLoadPaths.extract_relevant_paths(@root_path, "test") + @load_paths ||= Private::ApplicationLoadPaths.extract_relevant_paths(@root_path, "test") end def parallel? diff --git a/lib/packwerk/offense.rb b/lib/packwerk/offense.rb index 225a3e7f9..27bf8717c 100644 --- a/lib/packwerk/offense.rb +++ b/lib/packwerk/offense.rb @@ -8,7 +8,7 @@ class Offense extend T::Sig extend T::Helpers - sig { returns(T.nilable(Node::Location)) } + sig { returns(T.nilable(Private::Node::Location)) } attr_reader :location sig { returns(String) } @@ -18,7 +18,7 @@ class Offense attr_reader :message sig do - params(file: String, message: String, location: T.nilable(Node::Location)) + params(file: String, message: String, location: T.nilable(Private::Node::Location)) .void end def initialize(file:, message:, location: nil) diff --git a/lib/packwerk/private/offense_collection.rb b/lib/packwerk/offense_collection.rb similarity index 100% rename from lib/packwerk/private/offense_collection.rb rename to lib/packwerk/offense_collection.rb diff --git a/lib/packwerk/private.rb b/lib/packwerk/private.rb index efc9626fd..c7215009b 100644 --- a/lib/packwerk/private.rb +++ b/lib/packwerk/private.rb @@ -12,57 +12,28 @@ module Private autoload :ApplicationValidator autoload :AssociationInspector autoload :Cache - autoload :Configuration autoload :ConstantDiscovery autoload :ConstantNameInspector autoload :ConstNodeInspector autoload :ExtensionLoader autoload :FileProcessor autoload :FilesForProcessing - autoload :Graph autoload :Node autoload :NodeHelpers autoload :NodeProcessor autoload :NodeProcessorFactory autoload :NodeVisitor - autoload :OffenseCollection autoload :ParsedConstantDefinitions autoload :ParseRun autoload :ReferenceExtractor autoload :RunContext autoload :UnresolvedReference - module Formatters + class ApplicationValidator extend ActiveSupport::Autoload - autoload :ProgressFormatter + autoload :Helpers end - - private_constant :Formatters - - module Generators - extend ActiveSupport::Autoload - - autoload :ConfigurationFile - autoload :RootPackage - end - - private_constant :Generators - - module ReferenceChecking - extend ActiveSupport::Autoload - - autoload :ReferenceChecker - - module Checkers - extend ActiveSupport::Autoload - - autoload :DependencyChecker - autoload :PrivacyChecker - end - end - - private_constant :ReferenceChecking end private_constant :Private diff --git a/lib/packwerk/private/application_load_paths.rb b/lib/packwerk/private/application_load_paths.rb index ec5710e15..5c4d5d602 100644 --- a/lib/packwerk/private/application_load_paths.rb +++ b/lib/packwerk/private/application_load_paths.rb @@ -4,68 +4,70 @@ require "bundler" module Packwerk - # Extracts the load paths from the analyzed application so that we can map constant names to paths. - module ApplicationLoadPaths - class << self - extend T::Sig + module Private + # Extracts the load paths from the analyzed application so that we can map constant names to paths. + module ApplicationLoadPaths + class << self + extend T::Sig - 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 - relevant_paths = filter_relevant_paths(all_paths) - assert_load_paths_present(relevant_paths) - relative_path_strings(relevant_paths) - end + 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 + relevant_paths = filter_relevant_paths(all_paths) + assert_load_paths_present(relevant_paths) + relative_path_strings(relevant_paths) + end - sig { returns(T::Hash[String, Module]) } - def extract_application_autoload_paths - Rails.autoloaders.inject({}) do |h, loader| - h.merge(loader.dirs(namespaces: true)) + 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 - 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("**") + 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 + 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(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 + private - sig { params(root: String, environment: String).void } - def require_application(root, environment) - environment_file = "#{root}/config/environment" + sig { params(root: String, environment: String).void } + def require_application(root, environment) + environment_file = "#{root}/config/environment" - if File.file?("#{environment_file}.rb") - ENV["RAILS_ENV"] ||= environment + if File.file?("#{environment_file}.rb") + ENV["RAILS_ENV"] ||= environment - require environment_file - else - raise "A Rails application could not be found in #{root}" + require environment_file + else + raise "A Rails application could not be found in #{root}" + end 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 + 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 diff --git a/lib/packwerk/private/application_validator.rb b/lib/packwerk/private/application_validator.rb index 563a59712..8e6e3cb23 100644 --- a/lib/packwerk/private/application_validator.rb +++ b/lib/packwerk/private/application_validator.rb @@ -6,128 +6,130 @@ require "yaml" module Packwerk - # Checks the structure of the application and its packwerk configuration to make sure we can run a check and deliver - # correct results. - class ApplicationValidator - include Validator - extend T::Sig - - sig { params(package_set: PackageSet, configuration: Configuration).returns(Validator::Result) } - def check_all(package_set, configuration) - results = Validator.all.flat_map { |validator| validator.call(package_set, configuration) } - merge_results(results) - end + module Private + # Checks the structure of the application and its packwerk configuration to make sure we can run a check and deliver + # correct results. + class ApplicationValidator + include Validator + extend T::Sig + + sig { params(package_set: PackageSet, configuration: Configuration).returns(Validator::Result) } + def check_all(package_set, configuration) + results = Validator.all.flat_map { |validator| validator.call(package_set, configuration) } + merge_results(results) + end - sig { override.params(package_set: PackageSet, configuration: Configuration).returns(Validator::Result) } - def call(package_set, configuration) - results = [ - check_package_manifest_syntax(configuration), - check_application_structure(configuration), - check_package_manifest_paths(configuration), - check_root_package_exists(configuration), - ] + sig { override.params(package_set: PackageSet, configuration: Configuration).returns(Validator::Result) } + def call(package_set, configuration) + results = [ + check_package_manifest_syntax(configuration), + check_application_structure(configuration), + check_package_manifest_paths(configuration), + check_root_package_exists(configuration), + ] - merge_results(results, separator: "\n❓ ") - end + merge_results(results, separator: "\n❓ ") + end - sig { override.returns(T::Array[String]) } - def permitted_keys - [ - "metadata", - ] - end + sig { override.returns(T::Array[String]) } + def permitted_keys + [ + "metadata", + ] + end - sig { params(configuration: Configuration).returns(Validator::Result) } - def check_package_manifest_syntax(configuration) - errors = [] + sig { params(configuration: Configuration).returns(Validator::Result) } + def check_package_manifest_syntax(configuration) + errors = [] - package_manifests(configuration).each do |manifest| - hash = YAML.load_file(manifest) - next unless hash + package_manifests(configuration).each do |manifest| + hash = YAML.load_file(manifest) + next unless hash - known_keys = Validator.all.flat_map(&:permitted_keys) - unknown_keys = hash.keys - known_keys + known_keys = Validator.all.flat_map(&:permitted_keys) + unknown_keys = hash.keys - known_keys - unless unknown_keys.empty? - errors << "\tUnknown keys: #{unknown_keys.inspect} in #{manifest.inspect}" + unless unknown_keys.empty? + errors << "\tUnknown keys: #{unknown_keys.inspect} in #{manifest.inspect}" + end + end + + if errors.empty? + Validator::Result.new(ok: true) + else + merge_results( + errors.map { |error| Validator::Result.new(ok: false, error_value: error) }, + separator: "\n", + before_errors: "Malformed syntax in the following manifests:\n\n", + after_errors: "\n", + ) end end - if errors.empty? - Validator::Result.new(ok: true) - else - merge_results( - errors.map { |error| Validator::Result.new(ok: false, error_value: error) }, - separator: "\n", - before_errors: "Malformed syntax in the following manifests:\n\n", - after_errors: "\n", + sig { params(configuration: Configuration).returns(Validator::Result) } + def check_application_structure(configuration) + resolver = ConstantResolver.new( + root_path: configuration.root_path.to_s, + load_paths: configuration.load_paths ) - end - end - sig { params(configuration: Configuration).returns(Validator::Result) } - def check_application_structure(configuration) - resolver = ConstantResolver.new( - root_path: configuration.root_path.to_s, - load_paths: configuration.load_paths - ) - - begin - resolver.file_map - Validator::Result.new(ok: true) - rescue => e - Validator::Result.new(ok: false, error_value: e.message) + begin + resolver.file_map + Validator::Result.new(ok: true) + rescue => e + Validator::Result.new(ok: false, error_value: e.message) + end end - end - sig { params(configuration: Configuration).returns(Validator::Result) } - def check_package_manifest_paths(configuration) - all_package_manifests = package_manifests(configuration, "**/") - package_paths_package_manifests = package_manifests(configuration, package_glob(configuration)) + sig { params(configuration: Configuration).returns(Validator::Result) } + def check_package_manifest_paths(configuration) + all_package_manifests = package_manifests(configuration, "**/") + package_paths_package_manifests = package_manifests(configuration, package_glob(configuration)) - difference = all_package_manifests - package_paths_package_manifests + difference = all_package_manifests - package_paths_package_manifests - if difference.empty? - Validator::Result.new(ok: true) - else - Validator::Result.new( - ok: false, - error_value: <<~EOS - Expected package paths for all package.ymls to be specified, but paths were missing for the following manifests: + if difference.empty? + Validator::Result.new(ok: true) + else + Validator::Result.new( + ok: false, + error_value: <<~EOS + Expected package paths for all package.ymls to be specified, but paths were missing for the following manifests: - #{relative_paths(configuration, difference).join("\n")} - EOS - ) + #{relative_paths(configuration, difference).join("\n")} + EOS + ) + end end - end - sig { params(configuration: Configuration).returns(Validator::Result) } - def check_root_package_exists(configuration) - root_package_path = File.join(configuration.root_path, "package.yml") - all_packages_manifests = package_manifests(configuration, package_glob(configuration)) - - if all_packages_manifests.include?(root_package_path) - Validator::Result.new(ok: true) - else - Validator::Result.new( - ok: false, - error_value: <<~EOS - A root package does not exist. Create an empty `package.yml` at the root directory. - EOS - ) + sig { params(configuration: Configuration).returns(Validator::Result) } + def check_root_package_exists(configuration) + root_package_path = File.join(configuration.root_path, "package.yml") + all_packages_manifests = package_manifests(configuration, package_glob(configuration)) + + if all_packages_manifests.include?(root_package_path) + Validator::Result.new(ok: true) + else + Validator::Result.new( + ok: false, + error_value: <<~EOS + A root package does not exist. Create an empty `package.yml` at the root directory. + EOS + ) + end end - end - private + private - sig { params(list: T.untyped).returns(T.untyped) } - def format_yaml_strings(list) - list.sort.map { |p| "- \"#{p}\"" }.join("\n") - end + sig { params(list: T.untyped).returns(T.untyped) } + def format_yaml_strings(list) + list.sort.map { |p| "- \"#{p}\"" }.join("\n") + end - sig { params(configuration: Configuration, paths: T::Array[String]).returns(T::Array[Pathname]) } - def relative_paths(configuration, paths) - paths.map { |path| relative_path(configuration, path) } + sig { params(configuration: Configuration, paths: T::Array[String]).returns(T::Array[Pathname]) } + def relative_paths(configuration, paths) + paths.map { |path| relative_path(configuration, path) } + end end end end diff --git a/lib/packwerk/private/association_inspector.rb b/lib/packwerk/private/association_inspector.rb index 899ef8499..448637d36 100644 --- a/lib/packwerk/private/association_inspector.rb +++ b/lib/packwerk/private/association_inspector.rb @@ -2,72 +2,74 @@ # frozen_string_literal: true module Packwerk - # Extracts the implicit constant reference from an active record association - class AssociationInspector - extend T::Sig - include ConstantNameInspector + module Private + # Extracts the implicit constant reference from an active record association + class AssociationInspector + extend T::Sig + include ConstantNameInspector - CustomAssociations = T.type_alias { T.any(T::Array[Symbol], T::Set[Symbol]) } + CustomAssociations = T.type_alias { T.any(T::Array[Symbol], T::Set[Symbol]) } - RAILS_ASSOCIATIONS = T.let( - [ - :belongs_to, - :has_many, - :has_one, - :has_and_belongs_to_many, - ].to_set, - CustomAssociations - ) + RAILS_ASSOCIATIONS = T.let( + [ + :belongs_to, + :has_many, + :has_one, + :has_and_belongs_to_many, + ].to_set, + CustomAssociations + ) - sig { params(inflector: T.class_of(ActiveSupport::Inflector), custom_associations: CustomAssociations).void } - def initialize(inflector:, custom_associations: Set.new) - @inflector = inflector - @associations = T.let(RAILS_ASSOCIATIONS + custom_associations, CustomAssociations) - end + sig { params(inflector: T.class_of(ActiveSupport::Inflector), custom_associations: CustomAssociations).void } + def initialize(inflector:, custom_associations: Set.new) + @inflector = inflector + @associations = T.let(RAILS_ASSOCIATIONS + custom_associations, CustomAssociations) + end - sig do - override - .params(node: AST::Node, ancestors: T::Array[AST::Node]) - .returns(T.nilable(String)) - end - def constant_name_from_node(node, ancestors:) - return unless NodeHelpers.method_call?(node) - return unless association?(node) + sig do + override + .params(node: AST::Node, ancestors: T::Array[AST::Node]) + .returns(T.nilable(String)) + end + def constant_name_from_node(node, ancestors:) + return unless NodeHelpers.method_call?(node) + return unless association?(node) - arguments = NodeHelpers.method_arguments(node) - return unless (association_name = association_name(arguments)) + arguments = NodeHelpers.method_arguments(node) + return unless (association_name = association_name(arguments)) - if (class_name_node = custom_class_name(arguments)) - return unless NodeHelpers.string?(class_name_node) + if (class_name_node = custom_class_name(arguments)) + return unless NodeHelpers.string?(class_name_node) - NodeHelpers.literal_value(class_name_node) - else - @inflector.classify(association_name.to_s) + NodeHelpers.literal_value(class_name_node) + else + @inflector.classify(association_name.to_s) + end end - end - private + private - sig { params(node: AST::Node).returns(T::Boolean) } - def association?(node) - method_name = NodeHelpers.method_name(node) - @associations.include?(method_name) - end + sig { params(node: AST::Node).returns(T::Boolean) } + def association?(node) + method_name = NodeHelpers.method_name(node) + @associations.include?(method_name) + end - sig { params(arguments: T::Array[AST::Node]).returns(T.nilable(AST::Node)) } - def custom_class_name(arguments) - association_options = arguments.detect { |n| NodeHelpers.hash?(n) } - return unless association_options + sig { params(arguments: T::Array[AST::Node]).returns(T.nilable(AST::Node)) } + def custom_class_name(arguments) + association_options = arguments.detect { |n| NodeHelpers.hash?(n) } + return unless association_options - NodeHelpers.value_from_hash(association_options, :class_name) - end + NodeHelpers.value_from_hash(association_options, :class_name) + end - sig { params(arguments: T::Array[AST::Node]).returns(T.any(T.nilable(Symbol), T.nilable(String))) } - def association_name(arguments) - association_name_node = T.must(arguments[0]) - return unless NodeHelpers.symbol?(association_name_node) + sig { params(arguments: T::Array[AST::Node]).returns(T.any(T.nilable(Symbol), T.nilable(String))) } + def association_name(arguments) + association_name_node = T.must(arguments[0]) + return unless NodeHelpers.symbol?(association_name_node) - NodeHelpers.literal_value(association_name_node) + NodeHelpers.literal_value(association_name_node) + end end end end diff --git a/lib/packwerk/private/cache.rb b/lib/packwerk/private/cache.rb index fb21ca08e..1301af6af 100644 --- a/lib/packwerk/private/cache.rb +++ b/lib/packwerk/private/cache.rb @@ -4,173 +4,175 @@ require "digest" module Packwerk - class Cache - extend T::Sig - - class CacheContents < T::Struct + module Private + class Cache extend T::Sig - const :file_contents_digest, String - const :unresolved_references, T::Array[UnresolvedReference] - - class << self + class CacheContents < T::Struct extend T::Sig - sig { params(serialized_cache_contents: String).returns(CacheContents) } - def deserialize(serialized_cache_contents) - cache_contents_json = JSON.parse(serialized_cache_contents) - unresolved_references = cache_contents_json["unresolved_references"].map do |json| - UnresolvedReference.new( - constant_name: json["constant_name"], - namespace_path: json["namespace_path"], - relative_path: json["relative_path"], - source_location: Node::Location.new(json["source_location"]["line"], json["source_location"]["column"],) + const :file_contents_digest, String + const :unresolved_references, T::Array[UnresolvedReference] + + class << self + extend T::Sig + + sig { params(serialized_cache_contents: String).returns(CacheContents) } + def deserialize(serialized_cache_contents) + cache_contents_json = JSON.parse(serialized_cache_contents) + unresolved_references = cache_contents_json["unresolved_references"].map do |json| + UnresolvedReference.new( + constant_name: json["constant_name"], + namespace_path: json["namespace_path"], + relative_path: json["relative_path"], + source_location: Node::Location.new(json["source_location"]["line"], json["source_location"]["column"],) + ) + end + + CacheContents.new( + file_contents_digest: cache_contents_json["file_contents_digest"], + unresolved_references: unresolved_references, ) end + end - CacheContents.new( - file_contents_digest: cache_contents_json["file_contents_digest"], - unresolved_references: unresolved_references, - ) + sig { returns(String) } + def serialize + to_json end end - sig { returns(String) } - def serialize - to_json + CacheShape = T.type_alias do + T::Hash[ + String, + CacheContents + ] end - end - - CacheShape = T.type_alias do - T::Hash[ - String, - CacheContents - ] - end - sig { params(enable_cache: T::Boolean, cache_directory: Pathname, config_path: T.nilable(String)).void } - def initialize(enable_cache:, cache_directory:, config_path:) - @enable_cache = enable_cache - @cache = T.let({}, CacheShape) - @files_by_digest = T.let({}, T::Hash[String, String]) - @config_path = config_path - @cache_directory = cache_directory - - if @enable_cache - create_cache_directory! - bust_cache_if_packwerk_yml_has_changed! - bust_cache_if_inflections_have_changed! + sig { params(enable_cache: T::Boolean, cache_directory: Pathname, config_path: T.nilable(String)).void } + def initialize(enable_cache:, cache_directory:, config_path:) + @enable_cache = enable_cache + @cache = T.let({}, CacheShape) + @files_by_digest = T.let({}, T::Hash[String, String]) + @config_path = config_path + @cache_directory = cache_directory + + if @enable_cache + create_cache_directory! + bust_cache_if_packwerk_yml_has_changed! + bust_cache_if_inflections_have_changed! + end end - end - sig { void } - def bust_cache! - FileUtils.rm_rf(@cache_directory) - end + sig { void } + def bust_cache! + FileUtils.rm_rf(@cache_directory) + end - sig do - params( - file_path: String, - block: T.proc.returns(T::Array[UnresolvedReference]) - ).returns(T::Array[UnresolvedReference]) - end - def with_cache(file_path, &block) - return yield unless @enable_cache + sig do + params( + file_path: String, + block: T.proc.returns(T::Array[UnresolvedReference]) + ).returns(T::Array[UnresolvedReference]) + end + def with_cache(file_path, &block) + return yield unless @enable_cache - cache_location = @cache_directory.join(digest_for_string(file_path)) + cache_location = @cache_directory.join(digest_for_string(file_path)) - cache_contents = if cache_location.exist? - T.let(CacheContents.deserialize(cache_location.read), - CacheContents) - end + cache_contents = if cache_location.exist? + T.let(CacheContents.deserialize(cache_location.read), + CacheContents) + end - file_contents_digest = digest_for_file(file_path) + file_contents_digest = digest_for_file(file_path) - if !cache_contents.nil? && cache_contents.file_contents_digest == file_contents_digest - Debug.out("Cache hit for #{file_path}") + if !cache_contents.nil? && cache_contents.file_contents_digest == file_contents_digest + Debug.out("Cache hit for #{file_path}") - cache_contents.unresolved_references - else - Debug.out("Cache miss for #{file_path}") + cache_contents.unresolved_references + else + Debug.out("Cache miss for #{file_path}") - unresolved_references = yield + unresolved_references = yield - cache_contents = CacheContents.new( - file_contents_digest: file_contents_digest, - unresolved_references: unresolved_references, - ) - cache_location.write(cache_contents.serialize) + cache_contents = CacheContents.new( + file_contents_digest: file_contents_digest, + unresolved_references: unresolved_references, + ) + cache_location.write(cache_contents.serialize) - unresolved_references + unresolved_references + end end - end - sig { params(file: String).returns(String) } - def digest_for_file(file) - digest_for_string(File.read(file)) - end + sig { params(file: String).returns(String) } + def digest_for_file(file) + digest_for_string(File.read(file)) + end - sig { params(str: String).returns(String) } - def digest_for_string(str) - # MD5 appears to be the fastest - # https://gist.github.com/morimori/1330095 - Digest::MD5.hexdigest(str) - end + sig { params(str: String).returns(String) } + def digest_for_string(str) + # MD5 appears to be the fastest + # https://gist.github.com/morimori/1330095 + Digest::MD5.hexdigest(str) + end - sig { void } - def bust_cache_if_packwerk_yml_has_changed! - return nil if @config_path.nil? + sig { void } + def bust_cache_if_packwerk_yml_has_changed! + return nil if @config_path.nil? - bust_cache_if_contents_have_changed(File.read(@config_path), :packwerk_yml) - end + bust_cache_if_contents_have_changed(File.read(@config_path), :packwerk_yml) + end - sig { void } - def bust_cache_if_inflections_have_changed! - bust_cache_if_contents_have_changed(YAML.dump(ActiveSupport::Inflector.inflections), :inflections) - end + sig { void } + def bust_cache_if_inflections_have_changed! + bust_cache_if_contents_have_changed(YAML.dump(ActiveSupport::Inflector.inflections), :inflections) + end - sig { params(contents: String, contents_key: Symbol).void } - def bust_cache_if_contents_have_changed(contents, contents_key) - current_digest = digest_for_string(contents) - cached_digest_path = @cache_directory.join(contents_key.to_s) + sig { params(contents: String, contents_key: Symbol).void } + def bust_cache_if_contents_have_changed(contents, contents_key) + current_digest = digest_for_string(contents) + cached_digest_path = @cache_directory.join(contents_key.to_s) - if !cached_digest_path.exist? - # In this case, we have nothing cached - # We save the current digest. This way the next time we compare current digest to cached digest, - # we can accurately determine if we should bust the cache - cached_digest_path.write(current_digest) + if !cached_digest_path.exist? + # In this case, we have nothing cached + # We save the current digest. This way the next time we compare current digest to cached digest, + # we can accurately determine if we should bust the cache + cached_digest_path.write(current_digest) - nil - elsif cached_digest_path.read == current_digest - Debug.out("#{contents_key} contents have NOT changed, preserving cache") - else - Debug.out("#{contents_key} contents have changed, busting cache") + nil + elsif cached_digest_path.read == current_digest + Debug.out("#{contents_key} contents have NOT changed, preserving cache") + else + Debug.out("#{contents_key} contents have changed, busting cache") - bust_cache! - create_cache_directory! + bust_cache! + create_cache_directory! - cached_digest_path.write(current_digest) + cached_digest_path.write(current_digest) + end end - end - sig { void } - def create_cache_directory! - FileUtils.mkdir_p(@cache_directory) + sig { void } + def create_cache_directory! + FileUtils.mkdir_p(@cache_directory) + end end - end - class Debug - class << self - extend T::Sig + class Debug + class << self + extend T::Sig - sig { params(out: String).void } - def out(out) - if ENV["DEBUG_PACKWERK_CACHE"] - puts(out) + sig { params(out: String).void } + def out(out) + if ENV["DEBUG_PACKWERK_CACHE"] + puts(out) + end end - end - end - end + end + end - private_constant :Debug + private_constant :Debug + end end diff --git a/lib/packwerk/private/const_node_inspector.rb b/lib/packwerk/private/const_node_inspector.rb index eda29168d..d1103bc14 100644 --- a/lib/packwerk/private/const_node_inspector.rb +++ b/lib/packwerk/private/const_node_inspector.rb @@ -2,54 +2,56 @@ # frozen_string_literal: true module Packwerk - # Extracts a constant name from an AST node of type :const - class ConstNodeInspector - extend T::Sig - include ConstantNameInspector - - sig do - override - .params(node: AST::Node, ancestors: T::Array[AST::Node]) - .returns(T.nilable(String)) - end - def constant_name_from_node(node, ancestors:) - return nil unless NodeHelpers.constant?(node) - - parent = ancestors.first - - # Only process the root `const` node for namespaced constant references. For example, in the - # reference `Spam::Eggs::Thing`, we only process the const node associated with `Spam`. - return nil unless root_constant?(parent) - - if parent && constant_in_module_or_class_definition?(node, parent: parent) - fully_qualify_constant(ancestors) - else - begin - NodeHelpers.constant_name(node) - rescue NodeHelpers::TypeError - nil + module Private + # Extracts a constant name from an AST node of type :const + class ConstNodeInspector + extend T::Sig + include ConstantNameInspector + + sig do + override + .params(node: AST::Node, ancestors: T::Array[AST::Node]) + .returns(T.nilable(String)) + end + def constant_name_from_node(node, ancestors:) + return nil unless NodeHelpers.constant?(node) + + parent = ancestors.first + + # Only process the root `const` node for namespaced constant references. For example, in the + # reference `Spam::Eggs::Thing`, we only process the const node associated with `Spam`. + return nil unless root_constant?(parent) + + if parent && constant_in_module_or_class_definition?(node, parent: parent) + fully_qualify_constant(ancestors) + else + begin + NodeHelpers.constant_name(node) + rescue NodeHelpers::TypeError + nil + end end end - end - private + private - sig { params(parent: T.nilable(AST::Node)).returns(T::Boolean) } - def root_constant?(parent) - !(parent && NodeHelpers.constant?(parent)) - end + sig { params(parent: T.nilable(AST::Node)).returns(T::Boolean) } + def root_constant?(parent) + !(parent && NodeHelpers.constant?(parent)) + end - sig { params(node: AST::Node, parent: AST::Node).returns(T.nilable(T::Boolean)) } - def constant_in_module_or_class_definition?(node, parent:) - parent_name = NodeHelpers.module_name_from_definition(parent) - parent_name && parent_name == NodeHelpers.constant_name(node) - end + sig { params(node: AST::Node, parent: AST::Node).returns(T.nilable(T::Boolean)) } + def constant_in_module_or_class_definition?(node, parent:) + parent_name = NodeHelpers.module_name_from_definition(parent) + parent_name && parent_name == NodeHelpers.constant_name(node) + end - sig { params(ancestors: T::Array[AST::Node]).returns(String) } - def fully_qualify_constant(ancestors) - # We're defining a class with this name, in which case the constant is implicitly fully qualified by its - # enclosing namespace - "::" + NodeHelpers.parent_module_name(ancestors: ancestors) + sig { params(ancestors: T::Array[AST::Node]).returns(String) } + def fully_qualify_constant(ancestors) + # We're defining a class with this name, in which case the constant is implicitly fully qualified by its + # enclosing namespace + "::" + NodeHelpers.parent_module_name(ancestors: ancestors) + end end end end diff --git a/lib/packwerk/private/constant_discovery.rb b/lib/packwerk/private/constant_discovery.rb index 18be4afc0..19af4cea3 100644 --- a/lib/packwerk/private/constant_discovery.rb +++ b/lib/packwerk/private/constant_discovery.rb @@ -4,74 +4,76 @@ 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. - class ConstantDiscovery - extend T::Sig + module Private + # 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. + class ConstantDiscovery + extend T::Sig - ConstantContext = Struct.new(:name, :location, :package) + ConstantContext = Struct.new(:name, :location, :package) - # @param constant_resolver [ConstantResolver] - # @param packages [Packwerk::PackageSet] - sig do - params(constant_resolver: ConstantResolver, packages: Packwerk::PackageSet).void - end - def initialize(constant_resolver:, packages:) - @packages = packages - @resolver = constant_resolver - end + # @param constant_resolver [ConstantResolver] + # @param packages [Packwerk::PackageSet] + sig do + params(constant_resolver: ConstantResolver, packages: Packwerk::PackageSet).void + end + def initialize(constant_resolver:, packages:) + @packages = packages + @resolver = constant_resolver + end - # Get the package that owns a given file path. - # - # @param path [String] the file path - # - # @return [Packwerk::Package] the package that contains the given file, - # or nil if the path is not owned by any component - sig do - params( - path: String, - ).returns(Packwerk::Package) - end - def package_from_path(path) - @packages.package_from_path(path) - end + # Get the package that owns a given file path. + # + # @param path [String] the file path + # + # @return [Packwerk::Package] the package that contains the given file, + # or nil if the path is not owned by any component + sig do + params( + path: String, + ).returns(Packwerk::Package) + end + def package_from_path(path) + @packages.package_from_path(path) + end - # Analyze a constant via its name. - # If the constant is unresolved, we need the current namespace path to correctly infer its full name - # - # @param const_name [String] The unresolved constant's name. - # @param current_namespace_path [Array] (optional) The namespace of the context in which the constant is - # used, e.g. ["Apps", "Models"] for `Apps::Models`. Defaults to [] which means top level. - # @return [ConstantDiscovery::ConstantContext] - sig do - params( - const_name: String, - current_namespace_path: T.nilable(T::Array[String]), - ).returns(T.nilable(ConstantDiscovery::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) + # Analyze a constant via its name. + # If the constant is unresolved, we need the current namespace path to correctly infer its full name + # + # @param const_name [String] The unresolved constant's name. + # @param current_namespace_path [Array] (optional) The namespace of the context in which the constant is + # used, e.g. ["Apps", "Models"] for `Apps::Models`. Defaults to [] which means top level. + # @return [ConstantDiscovery::ConstantContext] + sig do + params( + const_name: String, + current_namespace_path: T.nilable(T::Array[String]), + ).returns(T.nilable(ConstantDiscovery::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) + end - return unless constant + return unless constant - package = @packages.package_from_path(constant.location) - ConstantContext.new( - constant.name, - constant.location, - package, - ) + package = @packages.package_from_path(constant.location) + ConstantContext.new( + constant.name, + constant.location, + package, + ) + end end end end diff --git a/lib/packwerk/private/constant_name_inspector.rb b/lib/packwerk/private/constant_name_inspector.rb index 70d93363d..9966d0c9c 100644 --- a/lib/packwerk/private/constant_name_inspector.rb +++ b/lib/packwerk/private/constant_name_inspector.rb @@ -4,18 +4,20 @@ require "ast" module Packwerk - # An interface describing an object that can extract a constant name from an AST node. - module ConstantNameInspector - extend T::Sig - extend T::Helpers + module Private + # An interface describing an object that can extract a constant name from an AST node. + module ConstantNameInspector + extend T::Sig + extend T::Helpers - interface! + interface! - sig do - abstract - .params(node: ::AST::Node, ancestors: T::Array[::AST::Node]) - .returns(T.nilable(String)) + sig do + abstract + .params(node: ::AST::Node, ancestors: T::Array[::AST::Node]) + .returns(T.nilable(String)) + end + def constant_name_from_node(node, ancestors:); end end - def constant_name_from_node(node, ancestors:); end end end diff --git a/lib/packwerk/private/extension_loader.rb b/lib/packwerk/private/extension_loader.rb index c54c884f2..2ddb40792 100644 --- a/lib/packwerk/private/extension_loader.rb +++ b/lib/packwerk/private/extension_loader.rb @@ -2,23 +2,23 @@ # frozen_string_literal: true module Packwerk - # This class handles loading extensions to packwerk using the `require` directive - # in the `packwerk.yml` configuration. - module ExtensionLoader - class << self - extend T::Sig - sig { params(require_directive: String, config_dir_path: String).void } - def load(require_directive, config_dir_path) - # We want to transform the require directive to behave differently - # if it's a specific local file being required versus a gem - if require_directive.start_with?(".") - require File.join(config_dir_path, require_directive) - else - require require_directive + module Private + # This class handles loading extensions to packwerk using the `require` directive + # in the `packwerk.yml` configuration. + module ExtensionLoader + class << self + extend T::Sig + sig { params(require_directive: String, config_dir_path: String).void } + def load(require_directive, config_dir_path) + # We want to transform the require directive to behave differently + # if it's a specific local file being required versus a gem + if require_directive.start_with?(".") + require File.join(config_dir_path, require_directive) + else + require require_directive + end end end end end - - private_constant :ExtensionLoader end diff --git a/lib/packwerk/private/file_processor.rb b/lib/packwerk/private/file_processor.rb index 6f2d4d961..22b238d6b 100644 --- a/lib/packwerk/private/file_processor.rb +++ b/lib/packwerk/private/file_processor.rb @@ -4,82 +4,84 @@ require "ast/node" module Packwerk - class FileProcessor - extend T::Sig - - class UnknownFileTypeResult < Offense + module Private + class FileProcessor extend T::Sig - sig { params(file: String).void } - def initialize(file:) - super(file: file, message: "unknown file type") - end - end - - sig do - params( - node_processor_factory: NodeProcessorFactory, - cache: Cache, - parser_factory: T.nilable(Parsers::Factory) - ).void - end - def initialize(node_processor_factory:, cache:, parser_factory: nil) - @node_processor_factory = node_processor_factory - @cache = cache - @parser_factory = T.let(parser_factory || Packwerk::Parsers::Factory.instance, Parsers::Factory) - end - - class ProcessedFile < T::Struct - const :unresolved_references, T::Array[UnresolvedReference], default: [] - const :offenses, T::Array[Offense], default: [] - end + class UnknownFileTypeResult < Offense + extend T::Sig - sig do - params(relative_file: String).returns(ProcessedFile) - end - def call(relative_file) - parser = parser_for(relative_file) - if parser.nil? - return ProcessedFile.new(offenses: [UnknownFileTypeResult.new(file: relative_file)]) + sig { params(file: String).void } + def initialize(file:) + super(file: file, message: "unknown file type") + end end - unresolved_references = @cache.with_cache(relative_file) do - node = parse_into_ast(relative_file, parser) - return ProcessedFile.new unless node + sig do + params( + node_processor_factory: NodeProcessorFactory, + cache: Cache, + parser_factory: T.nilable(Parsers::Factory) + ).void + end + def initialize(node_processor_factory:, cache:, parser_factory: nil) + @node_processor_factory = node_processor_factory + @cache = cache + @parser_factory = T.let(parser_factory || Packwerk::Parsers::Factory.instance, Parsers::Factory) + end - references_from_ast(node, relative_file) + class ProcessedFile < T::Struct + const :unresolved_references, T::Array[UnresolvedReference], default: [] + const :offenses, T::Array[Offense], default: [] end - ProcessedFile.new(unresolved_references: unresolved_references) - rescue Parsers::ParseError => e - ProcessedFile.new(offenses: [e.result]) - end + sig do + params(relative_file: String).returns(ProcessedFile) + end + def call(relative_file) + parser = parser_for(relative_file) + if parser.nil? + return ProcessedFile.new(offenses: [UnknownFileTypeResult.new(file: relative_file)]) + end + + unresolved_references = @cache.with_cache(relative_file) do + node = parse_into_ast(relative_file, parser) + return ProcessedFile.new unless node + + references_from_ast(node, relative_file) + end + + ProcessedFile.new(unresolved_references: unresolved_references) + rescue Parsers::ParseError => e + ProcessedFile.new(offenses: [e.result]) + end - private + private - sig do - params(node: Parser::AST::Node, relative_file: String).returns(T::Array[UnresolvedReference]) - end - def references_from_ast(node, relative_file) - references = [] + sig do + params(node: Parser::AST::Node, relative_file: String).returns(T::Array[UnresolvedReference]) + end + def references_from_ast(node, relative_file) + references = [] - node_processor = @node_processor_factory.for(relative_file: relative_file, node: node) - node_visitor = NodeVisitor.new(node_processor: node_processor) - node_visitor.visit(node, ancestors: [], result: references) + node_processor = @node_processor_factory.for(relative_file: relative_file, node: node) + node_visitor = NodeVisitor.new(node_processor: node_processor) + node_visitor.visit(node, ancestors: [], result: references) - references - end + references + end - sig { params(relative_file: String, parser: Parsers::ParserInterface).returns(T.untyped) } - def parse_into_ast(relative_file, parser) - File.open(relative_file, "r", nil, external_encoding: Encoding::UTF_8) do |file| - parser.call(io: file, file_path: relative_file) + sig { params(relative_file: String, parser: Parsers::ParserInterface).returns(T.untyped) } + def parse_into_ast(relative_file, parser) + File.open(relative_file, "r", nil, external_encoding: Encoding::UTF_8) do |file| + parser.call(io: file, file_path: relative_file) + end end - end - sig { params(file_path: String).returns(T.nilable(Parsers::ParserInterface)) } - def parser_for(file_path) - @parser_factory.for_path(file_path) + sig { params(file_path: String).returns(T.nilable(Parsers::ParserInterface)) } + def parser_for(file_path) + @parser_factory.for_path(file_path) + end end end end diff --git a/lib/packwerk/private/files_for_processing.rb b/lib/packwerk/private/files_for_processing.rb index 034f2207a..69a7fe826 100644 --- a/lib/packwerk/private/files_for_processing.rb +++ b/lib/packwerk/private/files_for_processing.rb @@ -2,113 +2,115 @@ # frozen_string_literal: true module Packwerk - class FilesForProcessing - extend T::Sig + module Private + class FilesForProcessing + extend T::Sig - RelativeFileSet = T.type_alias { T::Set[String] } + RelativeFileSet = T.type_alias { T::Set[String] } - class << self - extend T::Sig + class << self + extend T::Sig + + sig do + params( + relative_file_paths: T::Array[String], + configuration: Configuration, + ignore_nested_packages: T::Boolean + ).returns(FilesForProcessing) + end + def fetch(relative_file_paths:, configuration:, ignore_nested_packages: false) + new(relative_file_paths, configuration, ignore_nested_packages) + end + end sig do params( relative_file_paths: T::Array[String], configuration: Configuration, ignore_nested_packages: T::Boolean - ).returns(FilesForProcessing) + ).void end - def fetch(relative_file_paths:, configuration:, ignore_nested_packages: false) - new(relative_file_paths, configuration, ignore_nested_packages) + def initialize(relative_file_paths, configuration, ignore_nested_packages) + @relative_file_paths = relative_file_paths + @configuration = configuration + @ignore_nested_packages = ignore_nested_packages + @specified_files = T.let(nil, T.nilable(RelativeFileSet)) + @files = T.let(nil, T.nilable(RelativeFileSet)) end - end - sig do - params( - relative_file_paths: T::Array[String], - configuration: Configuration, - ignore_nested_packages: T::Boolean - ).void - end - def initialize(relative_file_paths, configuration, ignore_nested_packages) - @relative_file_paths = relative_file_paths - @configuration = configuration - @ignore_nested_packages = ignore_nested_packages - @specified_files = T.let(nil, T.nilable(RelativeFileSet)) - @files = T.let(nil, T.nilable(RelativeFileSet)) - end + sig { returns(RelativeFileSet) } + def files + @files ||= files_for_processing + end - sig { returns(RelativeFileSet) } - def files - @files ||= files_for_processing - end + sig { returns(T::Boolean) } + def files_specified? + specified_files.any? + end - sig { returns(T::Boolean) } - def files_specified? - specified_files.any? - end + private - private + sig { returns(RelativeFileSet) } + def files_for_processing + all_included_files = if specified_files.empty? + configured_included_files + else + configured_included_files & specified_files + end - sig { returns(RelativeFileSet) } - def files_for_processing - all_included_files = if specified_files.empty? - configured_included_files - else - configured_included_files & specified_files + all_included_files - configured_excluded_files end - all_included_files - configured_excluded_files - end + sig { returns(RelativeFileSet) } + def specified_files + @specified_files ||= Set.new( + @relative_file_paths.map do |relative_file_path| + if File.file?(relative_file_path) + relative_file_path + else + specified_included_files(relative_file_path) + end + end + ).flatten + end - sig { returns(RelativeFileSet) } - def specified_files - @specified_files ||= Set.new( - @relative_file_paths.map do |relative_file_path| - if File.file?(relative_file_path) - relative_file_path - else - specified_included_files(relative_file_path) + sig { params(relative_file_path: String).returns(RelativeFileSet) } + def specified_included_files(relative_file_path) + # Note, assuming include globs are always relative paths + relative_includes = @configuration.include + relative_files = Dir.glob([File.join(relative_file_path, "**", "*")]).select do |relative_path| + relative_includes.any? do |pattern| + File.fnmatch?(pattern, relative_path, File::FNM_EXTGLOB) end end - ).flatten - end - sig { params(relative_file_path: String).returns(RelativeFileSet) } - def specified_included_files(relative_file_path) - # Note, assuming include globs are always relative paths - relative_includes = @configuration.include - relative_files = Dir.glob([File.join(relative_file_path, "**", "*")]).select do |relative_path| - relative_includes.any? do |pattern| - File.fnmatch?(pattern, relative_path, File::FNM_EXTGLOB) + if @ignore_nested_packages + nested_packages_relative_file_paths = Dir.glob(File.join(relative_file_path, "*", "**", "package.yml")) + nested_packages_relative_globs = nested_packages_relative_file_paths.map do |npp| + npp.gsub("package.yml", "**/*") + end + nested_packages_relative_globs.each do |relative_glob| + relative_files -= Dir.glob(relative_glob) + end end - end - if @ignore_nested_packages - nested_packages_relative_file_paths = Dir.glob(File.join(relative_file_path, "*", "**", "package.yml")) - nested_packages_relative_globs = nested_packages_relative_file_paths.map do |npp| - npp.gsub("package.yml", "**/*") - end - nested_packages_relative_globs.each do |relative_glob| - relative_files -= Dir.glob(relative_glob) - end + Set.new(relative_files) end - Set.new(relative_files) - end - - sig { returns(RelativeFileSet) } - def configured_included_files - relative_files_for_globs(@configuration.include) - end + sig { returns(RelativeFileSet) } + def configured_included_files + relative_files_for_globs(@configuration.include) + end - sig { returns(RelativeFileSet) } - def configured_excluded_files - relative_files_for_globs(@configuration.exclude) - end + sig { returns(RelativeFileSet) } + def configured_excluded_files + relative_files_for_globs(@configuration.exclude) + end - sig { params(relative_globs: T::Array[String]).returns(RelativeFileSet) } - def relative_files_for_globs(relative_globs) - Set.new(relative_globs.flat_map { |glob| Dir[glob] }) + sig { params(relative_globs: T::Array[String]).returns(RelativeFileSet) } + def relative_files_for_globs(relative_globs) + Set.new(relative_globs.flat_map { |glob| Dir[glob] }) + end end end end diff --git a/lib/packwerk/private/node.rb b/lib/packwerk/private/node.rb index 5789486c2..dee18dab7 100644 --- a/lib/packwerk/private/node.rb +++ b/lib/packwerk/private/node.rb @@ -2,7 +2,9 @@ # frozen_string_literal: true module Packwerk - class Node - Location = Struct.new(:line, :column) + module Private + class Node + Location = Struct.new(:line, :column) + end end end diff --git a/lib/packwerk/private/node_helpers.rb b/lib/packwerk/private/node_helpers.rb index bc769294f..01dcb90c4 100644 --- a/lib/packwerk/private/node_helpers.rb +++ b/lib/packwerk/private/node_helpers.rb @@ -5,329 +5,331 @@ require "parser/ast/node" module Packwerk - # Convenience methods for working with Parser::AST::Node nodes. - module NodeHelpers - class TypeError < ArgumentError; end - - class << self - extend T::Sig - - sig { params(class_or_module_node: AST::Node).returns(String) } - def class_or_module_name(class_or_module_node) - case type_of(class_or_module_node) - when CLASS, MODULE - # (class (const nil :Foo) (const nil :Bar) (nil)) - # "class Foo < Bar; end" - # (module (const nil :Foo) (nil)) - # "module Foo; end" - identifier = class_or_module_node.children[0] - constant_name(identifier) - else - raise TypeError + module Private + # Convenience methods for working with Parser::AST::Node nodes. + module NodeHelpers + class TypeError < ArgumentError; end + + class << self + extend T::Sig + + sig { params(class_or_module_node: AST::Node).returns(String) } + def class_or_module_name(class_or_module_node) + case type_of(class_or_module_node) + when CLASS, MODULE + # (class (const nil :Foo) (const nil :Bar) (nil)) + # "class Foo < Bar; end" + # (module (const nil :Foo) (nil)) + # "module Foo; end" + identifier = class_or_module_node.children[0] + constant_name(identifier) + else + raise TypeError + end end - end - sig { params(constant_node: AST::Node).returns(String) } - def constant_name(constant_node) - case type_of(constant_node) - when CONSTANT_ROOT_NAMESPACE - "" - when CONSTANT, CONSTANT_ASSIGNMENT, SELF - # (const nil :Foo) - # "Foo" - # (const (cbase) :Foo) - # "::Foo" - # (const (lvar :a) :Foo) - # "a::Foo" - # (casgn nil :Foo (int 1)) - # "Foo = 1" - # (casgn (cbase) :Foo (int 1)) - # "::Foo = 1" - # (casgn (lvar :a) :Foo (int 1)) - # "a::Foo = 1" - # (casgn (self) :Foo (int 1)) - # "self::Foo = 1" - namespace, name = constant_node.children - - if namespace - [constant_name(namespace), name].join("::") + sig { params(constant_node: AST::Node).returns(String) } + def constant_name(constant_node) + case type_of(constant_node) + when CONSTANT_ROOT_NAMESPACE + "" + when CONSTANT, CONSTANT_ASSIGNMENT, SELF + # (const nil :Foo) + # "Foo" + # (const (cbase) :Foo) + # "::Foo" + # (const (lvar :a) :Foo) + # "a::Foo" + # (casgn nil :Foo (int 1)) + # "Foo = 1" + # (casgn (cbase) :Foo (int 1)) + # "::Foo = 1" + # (casgn (lvar :a) :Foo (int 1)) + # "a::Foo = 1" + # (casgn (self) :Foo (int 1)) + # "self::Foo = 1" + namespace, name = constant_node.children + + if namespace + [constant_name(namespace), name].join("::") + else + name.to_s + end else - name.to_s + raise TypeError end - else - raise TypeError end - end - sig { params(node: AST::Node).returns(T.untyped) } - def each_child(node) - if block_given? - node.children.each do |child| - yield child if child.is_a?(Parser::AST::Node) + sig { params(node: AST::Node).returns(T.untyped) } + def each_child(node) + if block_given? + node.children.each do |child| + yield child if child.is_a?(Parser::AST::Node) + end + else + enum_for(:each_child, node) end - else - enum_for(:each_child, node) end - end - sig { params(starting_node: AST::Node, ancestors: T::Array[AST::Node]).returns(T::Array[String]) } - def enclosing_namespace_path(starting_node, ancestors:) - ancestors.select { |n| [CLASS, MODULE].include?(type_of(n)) } - .each_with_object([]) do |node, namespace| - # when evaluating `class Child < Parent`, the const node for `Parent` is a child of the class - # node, so it'll be an ancestor, but `Parent` is not evaluated in the namespace of `Child`, so - # we need to skip it here - next if type_of(node) == CLASS && parent_class(node) == starting_node + sig { params(starting_node: AST::Node, ancestors: T::Array[AST::Node]).returns(T::Array[String]) } + def enclosing_namespace_path(starting_node, ancestors:) + ancestors.select { |n| [CLASS, MODULE].include?(type_of(n)) } + .each_with_object([]) do |node, namespace| + # when evaluating `class Child < Parent`, the const node for `Parent` is a child of the class + # node, so it'll be an ancestor, but `Parent` is not evaluated in the namespace of `Child`, so + # we need to skip it here + next if type_of(node) == CLASS && parent_class(node) == starting_node - namespace.prepend(class_or_module_name(node)) + namespace.prepend(class_or_module_name(node)) + end end - end - sig { params(string_or_symbol_node: AST::Node).returns(T.any(String, Symbol)) } - def literal_value(string_or_symbol_node) - case type_of(string_or_symbol_node) - when STRING, SYMBOL - # (str "foo") - # "'foo'" - # (sym :foo) - # ":foo" - string_or_symbol_node.children[0] - else - raise TypeError + sig { params(string_or_symbol_node: AST::Node).returns(T.any(String, Symbol)) } + def literal_value(string_or_symbol_node) + case type_of(string_or_symbol_node) + when STRING, SYMBOL + # (str "foo") + # "'foo'" + # (sym :foo) + # ":foo" + string_or_symbol_node.children[0] + else + raise TypeError + end end - end - sig { params(node: Parser::AST::Node).returns(Node::Location) } - def location(node) - location = node.location - Node::Location.new(location.line, location.column) - end + sig { params(node: Parser::AST::Node).returns(Node::Location) } + def location(node) + location = node.location + Node::Location.new(location.line, location.column) + end - sig { params(node: AST::Node).returns(T::Boolean) } - def constant?(node) - type_of(node) == CONSTANT - end + sig { params(node: AST::Node).returns(T::Boolean) } + def constant?(node) + type_of(node) == CONSTANT + end - sig { params(node: AST::Node).returns(T::Boolean) } - def constant_assignment?(node) - type_of(node) == CONSTANT_ASSIGNMENT - end + sig { params(node: AST::Node).returns(T::Boolean) } + def constant_assignment?(node) + type_of(node) == CONSTANT_ASSIGNMENT + end - sig { params(node: AST::Node).returns(T::Boolean) } - def class?(node) - type_of(node) == CLASS - end + sig { params(node: AST::Node).returns(T::Boolean) } + def class?(node) + type_of(node) == CLASS + end - sig { params(node: AST::Node).returns(T::Boolean) } - def method_call?(node) - type_of(node) == METHOD_CALL - end + sig { params(node: AST::Node).returns(T::Boolean) } + def method_call?(node) + type_of(node) == METHOD_CALL + end - sig { params(node: AST::Node).returns(T::Boolean) } - def hash?(node) - type_of(node) == HASH - end + sig { params(node: AST::Node).returns(T::Boolean) } + def hash?(node) + type_of(node) == HASH + end - sig { params(node: AST::Node).returns(T::Boolean) } - def string?(node) - type_of(node) == STRING - end + sig { params(node: AST::Node).returns(T::Boolean) } + def string?(node) + type_of(node) == STRING + end - sig { params(node: AST::Node).returns(T::Boolean) } - def symbol?(node) - type_of(node) == SYMBOL - end + sig { params(node: AST::Node).returns(T::Boolean) } + def symbol?(node) + type_of(node) == SYMBOL + end - sig { params(method_call_node: AST::Node).returns(T::Array[AST::Node]) } - def method_arguments(method_call_node) - raise TypeError unless method_call?(method_call_node) + sig { params(method_call_node: AST::Node).returns(T::Array[AST::Node]) } + def method_arguments(method_call_node) + raise TypeError unless method_call?(method_call_node) - # (send (lvar :foo) :bar (int 1)) - # "foo.bar(1)" - method_call_node.children.slice(2..-1) - end + # (send (lvar :foo) :bar (int 1)) + # "foo.bar(1)" + method_call_node.children.slice(2..-1) + end - sig { params(method_call_node: AST::Node).returns(Symbol) } - def method_name(method_call_node) - raise TypeError unless method_call?(method_call_node) + sig { params(method_call_node: AST::Node).returns(Symbol) } + def method_name(method_call_node) + raise TypeError unless method_call?(method_call_node) - # (send (lvar :foo) :bar (int 1)) - # "foo.bar(1)" - method_call_node.children[1] - end + # (send (lvar :foo) :bar (int 1)) + # "foo.bar(1)" + method_call_node.children[1] + end - sig { params(node: AST::Node).returns(T.nilable(String)) } - def module_name_from_definition(node) - case type_of(node) - when CLASS, MODULE - # "class My::Class; end" - # "module My::Module; end" - class_or_module_name(node) - when CONSTANT_ASSIGNMENT - # "My::Class = ..." - # "My::Module = ..." - rvalue = node.children.last - - case type_of(rvalue) - when METHOD_CALL - # "Class.new" - # "Module.new" - constant_name(node) if module_creation?(rvalue) - when BLOCK - # "Class.new do end" - # "Module.new do end" - constant_name(node) if module_creation?(method_call_node(rvalue)) + sig { params(node: AST::Node).returns(T.nilable(String)) } + def module_name_from_definition(node) + case type_of(node) + when CLASS, MODULE + # "class My::Class; end" + # "module My::Module; end" + class_or_module_name(node) + when CONSTANT_ASSIGNMENT + # "My::Class = ..." + # "My::Module = ..." + rvalue = node.children.last + + case type_of(rvalue) + when METHOD_CALL + # "Class.new" + # "Module.new" + constant_name(node) if module_creation?(rvalue) + when BLOCK + # "Class.new do end" + # "Module.new do end" + constant_name(node) if module_creation?(method_call_node(rvalue)) + end end end - end - sig { params(node: Parser::AST::Node).returns(T.nilable(Node::Location)) } - def name_location(node) - location = node.location + sig { params(node: Parser::AST::Node).returns(T.nilable(Node::Location)) } + def name_location(node) + location = node.location - if location.respond_to?(:name) - name = location.name - Node::Location.new(name.line, name.column) + if location.respond_to?(:name) + name = location.name + Node::Location.new(name.line, name.column) + end end - end - sig { params(class_node: AST::Node).returns(T.nilable(AST::Node)) } - def parent_class(class_node) - raise TypeError unless type_of(class_node) == CLASS + sig { params(class_node: AST::Node).returns(T.nilable(AST::Node)) } + def parent_class(class_node) + raise TypeError unless type_of(class_node) == CLASS - # (class (const nil :Foo) (const nil :Bar) (nil)) - # "class Foo < Bar; end" - class_node.children[1] - end + # (class (const nil :Foo) (const nil :Bar) (nil)) + # "class Foo < Bar; end" + class_node.children[1] + end - sig { params(ancestors: T::Array[AST::Node]).returns(String) } - def parent_module_name(ancestors:) - definitions = ancestors - .select { |n| [CLASS, MODULE, CONSTANT_ASSIGNMENT, BLOCK].include?(type_of(n)) } + sig { params(ancestors: T::Array[AST::Node]).returns(String) } + def parent_module_name(ancestors:) + definitions = ancestors + .select { |n| [CLASS, MODULE, CONSTANT_ASSIGNMENT, BLOCK].include?(type_of(n)) } - names = definitions.map do |definition| - name_part_from_definition(definition) - end.compact + names = definitions.map do |definition| + name_part_from_definition(definition) + end.compact - names.empty? ? "Object" : names.reverse.join("::") - end + names.empty? ? "Object" : names.reverse.join("::") + end - sig { params(hash_node: AST::Node, key: Symbol).returns(T.untyped) } - def value_from_hash(hash_node, key) - raise TypeError unless hash?(hash_node) + sig { params(hash_node: AST::Node, key: Symbol).returns(T.untyped) } + def value_from_hash(hash_node, key) + raise TypeError unless hash?(hash_node) - pair = hash_pairs(hash_node).detect { |pair_node| literal_value(hash_pair_key(pair_node)) == key } - hash_pair_value(pair) if pair - end + pair = hash_pairs(hash_node).detect { |pair_node| literal_value(hash_pair_key(pair_node)) == key } + hash_pair_value(pair) if pair + end - private - - BLOCK = :block - CLASS = :class - CONSTANT = :const - CONSTANT_ASSIGNMENT = :casgn - CONSTANT_ROOT_NAMESPACE = :cbase - HASH = :hash - HASH_PAIR = :pair - METHOD_CALL = :send - MODULE = :module - SELF = :self - STRING = :str - SYMBOL = :sym - - private_constant( - :BLOCK, :CLASS, :CONSTANT, :CONSTANT_ASSIGNMENT, :CONSTANT_ROOT_NAMESPACE, :HASH, :HASH_PAIR, :METHOD_CALL, - :MODULE, :SELF, :STRING, :SYMBOL, - ) - - sig { params(node: AST::Node).returns(Symbol) } - def type_of(node) - node.type - end + private + + BLOCK = :block + CLASS = :class + CONSTANT = :const + CONSTANT_ASSIGNMENT = :casgn + CONSTANT_ROOT_NAMESPACE = :cbase + HASH = :hash + HASH_PAIR = :pair + METHOD_CALL = :send + MODULE = :module + SELF = :self + STRING = :str + SYMBOL = :sym + + private_constant( + :BLOCK, :CLASS, :CONSTANT, :CONSTANT_ASSIGNMENT, :CONSTANT_ROOT_NAMESPACE, :HASH, :HASH_PAIR, :METHOD_CALL, + :MODULE, :SELF, :STRING, :SYMBOL, + ) + + sig { params(node: AST::Node).returns(Symbol) } + def type_of(node) + node.type + end - sig { params(hash_pair_node: AST::Node).returns(T.untyped) } - def hash_pair_key(hash_pair_node) - raise TypeError unless type_of(hash_pair_node) == HASH_PAIR + sig { params(hash_pair_node: AST::Node).returns(T.untyped) } + def hash_pair_key(hash_pair_node) + raise TypeError unless type_of(hash_pair_node) == HASH_PAIR - # (pair (int 1) (int 2)) - # "1 => 2" - # (pair (sym :answer) (int 42)) - # "answer: 42" - hash_pair_node.children[0] - end + # (pair (int 1) (int 2)) + # "1 => 2" + # (pair (sym :answer) (int 42)) + # "answer: 42" + hash_pair_node.children[0] + end - sig { params(hash_pair_node: AST::Node).returns(T.untyped) } - def hash_pair_value(hash_pair_node) - raise TypeError unless type_of(hash_pair_node) == HASH_PAIR + sig { params(hash_pair_node: AST::Node).returns(T.untyped) } + def hash_pair_value(hash_pair_node) + raise TypeError unless type_of(hash_pair_node) == HASH_PAIR - # (pair (int 1) (int 2)) - # "1 => 2" - # (pair (sym :answer) (int 42)) - # "answer: 42" - hash_pair_node.children[1] - end + # (pair (int 1) (int 2)) + # "1 => 2" + # (pair (sym :answer) (int 42)) + # "answer: 42" + hash_pair_node.children[1] + end - sig { params(hash_node: AST::Node).returns(T::Array[AST::Node]) } - def hash_pairs(hash_node) - raise TypeError unless hash?(hash_node) + sig { params(hash_node: AST::Node).returns(T::Array[AST::Node]) } + def hash_pairs(hash_node) + raise TypeError unless hash?(hash_node) - # (hash (pair (int 1) (int 2)) (pair (int 3) (int 4))) - # "{1 => 2, 3 => 4}" - hash_node.children.select { |n| type_of(n) == HASH_PAIR } - end + # (hash (pair (int 1) (int 2)) (pair (int 3) (int 4))) + # "{1 => 2, 3 => 4}" + hash_node.children.select { |n| type_of(n) == HASH_PAIR } + end - sig { params(block_node: AST::Node).returns(AST::Node) } - def method_call_node(block_node) - raise TypeError unless type_of(block_node) == BLOCK + sig { params(block_node: AST::Node).returns(AST::Node) } + def method_call_node(block_node) + raise TypeError unless type_of(block_node) == BLOCK - # (block (send (lvar :foo) :bar) (args) (int 42)) - # "foo.bar do 42 end" - block_node.children[0] - end + # (block (send (lvar :foo) :bar) (args) (int 42)) + # "foo.bar do 42 end" + block_node.children[0] + end - sig { params(node: AST::Node).returns(T::Boolean) } - def module_creation?(node) - # "Class.new" - # "Module.new" - method_call?(node) && - dynamic_class_creation?(receiver(node)) && - method_name(node) == :new - end + sig { params(node: AST::Node).returns(T::Boolean) } + def module_creation?(node) + # "Class.new" + # "Module.new" + method_call?(node) && + dynamic_class_creation?(receiver(node)) && + method_name(node) == :new + end - sig { params(node: T.nilable(AST::Node)).returns(T::Boolean) } - def dynamic_class_creation?(node) - !!node && - constant?(node) && - ["Class", "Module"].include?(constant_name(node)) - end + sig { params(node: T.nilable(AST::Node)).returns(T::Boolean) } + def dynamic_class_creation?(node) + !!node && + constant?(node) && + ["Class", "Module"].include?(constant_name(node)) + end - sig { params(node: AST::Node).returns(T.nilable(String)) } - def name_from_block_definition(node) - if method_name(method_call_node(node)) == :class_eval - receiver = receiver(node) - constant_name(receiver) if receiver && constant?(receiver) + sig { params(node: AST::Node).returns(T.nilable(String)) } + def name_from_block_definition(node) + if method_name(method_call_node(node)) == :class_eval + receiver = receiver(node) + constant_name(receiver) if receiver && constant?(receiver) + end end - end - sig { params(node: AST::Node).returns(T.nilable(String)) } - def name_part_from_definition(node) - case type_of(node) - when CLASS, MODULE, CONSTANT_ASSIGNMENT - module_name_from_definition(node) - when BLOCK - name_from_block_definition(node) + sig { params(node: AST::Node).returns(T.nilable(String)) } + def name_part_from_definition(node) + case type_of(node) + when CLASS, MODULE, CONSTANT_ASSIGNMENT + module_name_from_definition(node) + when BLOCK + name_from_block_definition(node) + end end - end - sig { params(method_call_or_block_node: AST::Node).returns(T.nilable(AST::Node)) } - def receiver(method_call_or_block_node) - case type_of(method_call_or_block_node) - when METHOD_CALL - method_call_or_block_node.children[0] - when BLOCK - receiver(method_call_node(method_call_or_block_node)) - else - raise TypeError + sig { params(method_call_or_block_node: AST::Node).returns(T.nilable(AST::Node)) } + def receiver(method_call_or_block_node) + case type_of(method_call_or_block_node) + when METHOD_CALL + method_call_or_block_node.children[0] + when BLOCK + receiver(method_call_node(method_call_or_block_node)) + else + raise TypeError + end end end end diff --git a/lib/packwerk/private/node_processor.rb b/lib/packwerk/private/node_processor.rb index e56a6ad24..0352c97ae 100644 --- a/lib/packwerk/private/node_processor.rb +++ b/lib/packwerk/private/node_processor.rb @@ -2,31 +2,33 @@ # frozen_string_literal: true module Packwerk - # Processes a single node in an abstract syntax tree (AST) using the provided checkers. - class NodeProcessor - extend T::Sig + module Private + # Processes a single node in an abstract syntax tree (AST) using the provided checkers. + class NodeProcessor + extend T::Sig - sig do - params( - reference_extractor: ReferenceExtractor, - relative_file: String, - ).void - end - def initialize(reference_extractor:, relative_file:) - @reference_extractor = reference_extractor - @relative_file = relative_file - end + sig do + params( + reference_extractor: ReferenceExtractor, + relative_file: String, + ).void + end + def initialize(reference_extractor:, relative_file:) + @reference_extractor = reference_extractor + @relative_file = relative_file + end - sig do - params( - node: Parser::AST::Node, - ancestors: T::Array[Parser::AST::Node] - ).returns(T.nilable(UnresolvedReference)) - end - def call(node, ancestors) - return unless NodeHelpers.method_call?(node) || NodeHelpers.constant?(node) + sig do + params( + node: Parser::AST::Node, + ancestors: T::Array[Parser::AST::Node] + ).returns(T.nilable(UnresolvedReference)) + end + def call(node, ancestors) + return unless NodeHelpers.method_call?(node) || NodeHelpers.constant?(node) - @reference_extractor.reference_from_node(node, ancestors: ancestors, relative_file: @relative_file) + @reference_extractor.reference_from_node(node, ancestors: ancestors, relative_file: @relative_file) + end end end end diff --git a/lib/packwerk/private/node_processor_factory.rb b/lib/packwerk/private/node_processor_factory.rb index e40ba884f..52c0edf7a 100644 --- a/lib/packwerk/private/node_processor_factory.rb +++ b/lib/packwerk/private/node_processor_factory.rb @@ -2,30 +2,32 @@ # frozen_string_literal: true module Packwerk - class NodeProcessorFactory < T::Struct - extend T::Sig + module Private + class NodeProcessorFactory < T::Struct + extend T::Sig - const :root_path, String - const :context_provider, ConstantDiscovery - const :constant_name_inspectors, T::Array[ConstantNameInspector] + const :root_path, String + const :context_provider, ConstantDiscovery + const :constant_name_inspectors, T::Array[ConstantNameInspector] - sig { params(relative_file: String, node: AST::Node).returns(NodeProcessor) } - def for(relative_file:, node:) - NodeProcessor.new( - reference_extractor: reference_extractor(node: node), - relative_file: relative_file, - ) - end + sig { params(relative_file: String, node: AST::Node).returns(NodeProcessor) } + def for(relative_file:, node:) + NodeProcessor.new( + reference_extractor: reference_extractor(node: node), + relative_file: relative_file, + ) + end - private + private - sig { params(node: AST::Node).returns(ReferenceExtractor) } - def reference_extractor(node:) - ReferenceExtractor.new( - constant_name_inspectors: constant_name_inspectors, - root_node: node, - root_path: root_path, - ) + sig { params(node: AST::Node).returns(ReferenceExtractor) } + def reference_extractor(node:) + ReferenceExtractor.new( + constant_name_inspectors: constant_name_inspectors, + root_node: node, + root_path: root_path, + ) + end end end end diff --git a/lib/packwerk/private/node_visitor.rb b/lib/packwerk/private/node_visitor.rb index 4a16faf57..1457dd3f6 100644 --- a/lib/packwerk/private/node_visitor.rb +++ b/lib/packwerk/private/node_visitor.rb @@ -2,22 +2,24 @@ # frozen_string_literal: true module Packwerk - # Visits all nodes of an AST, processing them using a given node processor. - class NodeVisitor - extend T::Sig + module Private + # Visits all nodes of an AST, processing them using a given node processor. + class NodeVisitor + extend T::Sig - sig { params(node_processor: NodeProcessor).void } - def initialize(node_processor:) - @node_processor = node_processor - end + sig { params(node_processor: NodeProcessor).void } + def initialize(node_processor:) + @node_processor = node_processor + end - def visit(node, ancestors:, result:) - reference = @node_processor.call(node, ancestors) - result << reference if reference + def visit(node, ancestors:, result:) + reference = @node_processor.call(node, ancestors) + result << reference if reference - child_ancestors = [node] + ancestors - NodeHelpers.each_child(node) do |child| - visit(child, ancestors: child_ancestors, result: result) + child_ancestors = [node] + ancestors + NodeHelpers.each_child(node) do |child| + visit(child, ancestors: child_ancestors, result: result) + end end end end diff --git a/lib/packwerk/private/parse_run.rb b/lib/packwerk/private/parse_run.rb index b4f209133..26416a57c 100644 --- a/lib/packwerk/private/parse_run.rb +++ b/lib/packwerk/private/parse_run.rb @@ -4,122 +4,124 @@ require "parallel" module Packwerk - class ParseRun - extend T::Sig + module Private + class ParseRun + extend T::Sig - ProcessFileProc = T.type_alias do - T.proc.params(path: String).returns(T::Array[Offense]) - end - - sig do - params( - relative_file_set: FilesForProcessing::RelativeFileSet, - configuration: Configuration, - file_set_specified: T::Boolean, - offenses_formatter: T.nilable(OffensesFormatter), - progress_formatter: Formatters::ProgressFormatter, - ).void - end - def initialize( - relative_file_set:, - configuration:, - file_set_specified: false, - offenses_formatter: nil, - progress_formatter: Formatters::ProgressFormatter.new(StringIO.new) - ) - - @configuration = configuration - @progress_formatter = progress_formatter - @offenses_formatter = T.let(offenses_formatter || configuration.offenses_formatter, Packwerk::OffensesFormatter) - @relative_file_set = relative_file_set - @file_set_specified = file_set_specified - end - - sig { returns(Cli::Result) } - def update_todo - if @file_set_specified - message = <<~MSG.squish - ⚠️ update-todo must be called without any file arguments. - MSG + ProcessFileProc = T.type_alias do + T.proc.params(path: String).returns(T::Array[Offense]) + end - return Cli::Result.new(message: message, status: false) + sig do + params( + relative_file_set: FilesForProcessing::RelativeFileSet, + configuration: Configuration, + file_set_specified: T::Boolean, + offenses_formatter: T.nilable(OffensesFormatter), + progress_formatter: Formatters::ProgressFormatter, + ).void + end + def initialize( + relative_file_set:, + configuration:, + file_set_specified: false, + offenses_formatter: nil, + progress_formatter: Formatters::ProgressFormatter.new(StringIO.new) + ) + + @configuration = configuration + @progress_formatter = progress_formatter + @offenses_formatter = T.let(offenses_formatter || configuration.offenses_formatter, Packwerk::OffensesFormatter) + @relative_file_set = relative_file_set + @file_set_specified = file_set_specified end - run_context = RunContext.from_configuration(@configuration) - offense_collection = find_offenses(run_context) - offense_collection.persist_package_todo_files(run_context.package_set) + sig { returns(Cli::Result) } + def update_todo + if @file_set_specified + message = <<~MSG.squish + ⚠️ update-todo must be called without any file arguments. + MSG - message = <<~EOS - #{@offenses_formatter.show_offenses(offense_collection.errors)} - ✅ `package_todo.yml` has been updated. - EOS + return Cli::Result.new(message: message, status: false) + end - Cli::Result.new(message: message, status: offense_collection.errors.empty?) - end + run_context = RunContext.from_configuration(@configuration) + offense_collection = find_offenses(run_context) + offense_collection.persist_package_todo_files(run_context.package_set) + + message = <<~EOS + #{@offenses_formatter.show_offenses(offense_collection.errors)} + ✅ `package_todo.yml` has been updated. + EOS - sig { returns(Cli::Result) } - def check - run_context = RunContext.from_configuration(@configuration) - offense_collection = find_offenses(run_context, show_errors: true) + Cli::Result.new(message: message, status: offense_collection.errors.empty?) + end - messages = [ - @offenses_formatter.show_offenses(offense_collection.outstanding_offenses), - @offenses_formatter.show_stale_violations(offense_collection, @relative_file_set), - @offenses_formatter.show_strict_mode_violations(offense_collection.strict_mode_violations), - ] + sig { returns(Cli::Result) } + def check + run_context = RunContext.from_configuration(@configuration) + offense_collection = find_offenses(run_context, show_errors: true) - result_status = offense_collection.outstanding_offenses.empty? && - !offense_collection.stale_violations?(@relative_file_set) && offense_collection.strict_mode_violations.empty? + messages = [ + @offenses_formatter.show_offenses(offense_collection.outstanding_offenses), + @offenses_formatter.show_stale_violations(offense_collection, @relative_file_set), + @offenses_formatter.show_strict_mode_violations(offense_collection.strict_mode_violations), + ] - Cli::Result.new(message: messages.select(&:present?).join("\n") + "\n", status: result_status) - end + result_status = offense_collection.outstanding_offenses.empty? && + !offense_collection.stale_violations?(@relative_file_set) && offense_collection.strict_mode_violations.empty? - private + Cli::Result.new(message: messages.select(&:present?).join("\n") + "\n", status: result_status) + end - sig { params(run_context: RunContext, show_errors: T::Boolean).returns(OffenseCollection) } - def find_offenses(run_context, show_errors: false) - offense_collection = OffenseCollection.new(@configuration.root_path) - all_offenses = T.let([], T::Array[Offense]) - process_file = T.let(->(relative_file) do - run_context.process_file(relative_file: relative_file).tap do |offenses| - failed = show_errors && offenses.any? { |offense| !offense_collection.listed?(offense) } - update_progress(failed: failed) + private + + sig { params(run_context: RunContext, show_errors: T::Boolean).returns(OffenseCollection) } + def find_offenses(run_context, show_errors: false) + offense_collection = OffenseCollection.new(@configuration.root_path) + all_offenses = T.let([], T::Array[Offense]) + process_file = T.let(->(relative_file) do + run_context.process_file(relative_file: relative_file).tap do |offenses| + failed = show_errors && offenses.any? { |offense| !offense_collection.listed?(offense) } + update_progress(failed: failed) + end + end, ProcessFileProc) + + @progress_formatter.started_inspection(@relative_file_set) do + all_offenses = if @configuration.parallel? + Parallel.flat_map(@relative_file_set, &process_file) + else + serial_find_offenses(&process_file) + end end - end, ProcessFileProc) - @progress_formatter.started_inspection(@relative_file_set) do - all_offenses = if @configuration.parallel? - Parallel.flat_map(@relative_file_set, &process_file) - else - serial_find_offenses(&process_file) - end + all_offenses.each { |offense| offense_collection.add_offense(offense) } + offense_collection end - all_offenses.each { |offense| offense_collection.add_offense(offense) } - offense_collection - end - - sig { params(block: ProcessFileProc).returns(T::Array[Offense]) } - def serial_find_offenses(&block) - all_offenses = T.let([], T::Array[Offense]) - begin - @relative_file_set.each do |relative_file| - offenses = yield(relative_file) - all_offenses.concat(offenses) + sig { params(block: ProcessFileProc).returns(T::Array[Offense]) } + def serial_find_offenses(&block) + all_offenses = T.let([], T::Array[Offense]) + begin + @relative_file_set.each do |relative_file| + offenses = yield(relative_file) + all_offenses.concat(offenses) + end + rescue Interrupt + @progress_formatter.interrupted + all_offenses end - rescue Interrupt - @progress_formatter.interrupted all_offenses end - all_offenses - end - sig { params(failed: T::Boolean).void } - def update_progress(failed: false) - if failed - @progress_formatter.mark_as_failed - else - @progress_formatter.mark_as_inspected + sig { params(failed: T::Boolean).void } + def update_progress(failed: false) + if failed + @progress_formatter.mark_as_failed + else + @progress_formatter.mark_as_inspected + end end end end diff --git a/lib/packwerk/private/parsed_constant_definitions.rb b/lib/packwerk/private/parsed_constant_definitions.rb index 589814a1f..924914a5e 100644 --- a/lib/packwerk/private/parsed_constant_definitions.rb +++ b/lib/packwerk/private/parsed_constant_definitions.rb @@ -4,61 +4,63 @@ require "ast/node" module Packwerk - # A collection of constant definitions parsed from an Abstract Syntax Tree (AST). - class ParsedConstantDefinitions - class << self - # What fully qualified constants can this constant refer to in this context? - def reference_qualifications(constant_name, namespace_path:) - return [constant_name] if constant_name.start_with?("::") + module Private + # A collection of constant definitions parsed from an Abstract Syntax Tree (AST). + class ParsedConstantDefinitions + class << self + # What fully qualified constants can this constant refer to in this context? + def reference_qualifications(constant_name, namespace_path:) + return [constant_name] if constant_name.start_with?("::") - resolved_constant_name = "::#{constant_name}" + resolved_constant_name = "::#{constant_name}" - possible_namespaces = namespace_path.each_with_object([""]) do |current, acc| - acc << "#{acc.last}::#{current}" if acc.last && current - end + possible_namespaces = namespace_path.each_with_object([""]) do |current, acc| + acc << "#{acc.last}::#{current}" if acc.last && current + end - possible_namespaces.map { |namespace| namespace + resolved_constant_name } + possible_namespaces.map { |namespace| namespace + resolved_constant_name } + end end - end - def initialize(root_node:) - @local_definitions = {} + def initialize(root_node:) + @local_definitions = {} - collect_local_definitions_from_root(root_node) if root_node - end + collect_local_definitions_from_root(root_node) if root_node + end - def local_reference?(constant_name, location: nil, namespace_path: []) - qualifications = self.class.reference_qualifications(constant_name, namespace_path: namespace_path) + def local_reference?(constant_name, location: nil, namespace_path: []) + qualifications = self.class.reference_qualifications(constant_name, namespace_path: namespace_path) - qualifications.any? do |name| - @local_definitions[name] && - @local_definitions[name] != location + qualifications.any? do |name| + @local_definitions[name] && + @local_definitions[name] != location + end end - end - private + private + + def collect_local_definitions_from_root(node, current_namespace_path = []) + if NodeHelpers.constant_assignment?(node) + add_definition(NodeHelpers.constant_name(node), current_namespace_path, NodeHelpers.name_location(node)) + elsif NodeHelpers.module_name_from_definition(node) + # handle compact constant nesting (e.g. "module Sales::Order") + tempnode = node + while (tempnode = NodeHelpers.each_child(tempnode).find { |n| NodeHelpers.constant?(n) }) + add_definition(NodeHelpers.constant_name(tempnode), current_namespace_path, + NodeHelpers.name_location(tempnode)) + end - def collect_local_definitions_from_root(node, current_namespace_path = []) - if NodeHelpers.constant_assignment?(node) - add_definition(NodeHelpers.constant_name(node), current_namespace_path, NodeHelpers.name_location(node)) - elsif NodeHelpers.module_name_from_definition(node) - # handle compact constant nesting (e.g. "module Sales::Order") - tempnode = node - while (tempnode = NodeHelpers.each_child(tempnode).find { |n| NodeHelpers.constant?(n) }) - add_definition(NodeHelpers.constant_name(tempnode), current_namespace_path, - NodeHelpers.name_location(tempnode)) + current_namespace_path += NodeHelpers.class_or_module_name(node).split("::") end - current_namespace_path += NodeHelpers.class_or_module_name(node).split("::") + NodeHelpers.each_child(node) { |child| collect_local_definitions_from_root(child, current_namespace_path) } end - NodeHelpers.each_child(node) { |child| collect_local_definitions_from_root(child, current_namespace_path) } - end + def add_definition(constant_name, current_namespace_path, location) + resolved_constant = [""].concat(current_namespace_path).push(constant_name).join("::") - def add_definition(constant_name, current_namespace_path, location) - resolved_constant = [""].concat(current_namespace_path).push(constant_name).join("::") - - @local_definitions[resolved_constant] = location + @local_definitions[resolved_constant] = location + end end end end diff --git a/lib/packwerk/private/reference_extractor.rb b/lib/packwerk/private/reference_extractor.rb index 18ec0d0a3..88c2552a1 100644 --- a/lib/packwerk/private/reference_extractor.rb +++ b/lib/packwerk/private/reference_extractor.rb @@ -2,127 +2,129 @@ # frozen_string_literal: true module Packwerk - # Extracts a possible constant reference from a given AST node. - class ReferenceExtractor - extend T::Sig - - class << self + module Private + # Extracts a possible constant reference from a given AST node. + class ReferenceExtractor extend T::Sig - sig do - params( - unresolved_references: T::Array[UnresolvedReference], - context_provider: ConstantDiscovery - ).returns(T::Array[Reference]) - end - def get_fully_qualified_references_from(unresolved_references, context_provider) - fully_qualified_references = T.let([], T::Array[Reference]) + class << self + extend T::Sig - unresolved_references.each do |unresolved_references_or_offense| - unresolved_reference = unresolved_references_or_offense + sig do + params( + unresolved_references: T::Array[UnresolvedReference], + context_provider: ConstantDiscovery + ).returns(T::Array[Reference]) + end + def get_fully_qualified_references_from(unresolved_references, context_provider) + fully_qualified_references = T.let([], T::Array[Reference]) - constant = - context_provider.context_for( - unresolved_reference.constant_name, - current_namespace_path: unresolved_reference.namespace_path - ) + unresolved_references.each do |unresolved_references_or_offense| + unresolved_reference = unresolved_references_or_offense - next if constant.nil? + constant = + context_provider.context_for( + unresolved_reference.constant_name, + current_namespace_path: unresolved_reference.namespace_path + ) - package_for_constant = constant.package + next if constant.nil? - next if package_for_constant.nil? + package_for_constant = constant.package - source_package = context_provider.package_from_path(unresolved_reference.relative_path) + next if package_for_constant.nil? - next if source_package == package_for_constant + source_package = context_provider.package_from_path(unresolved_reference.relative_path) - fully_qualified_references << Reference.new( - package: source_package, - relative_path: unresolved_reference.relative_path, - constant: constant, - source_location: unresolved_reference.source_location, - ) - end + next if source_package == package_for_constant - fully_qualified_references + fully_qualified_references << Reference.new( + package: source_package, + relative_path: unresolved_reference.relative_path, + constant: constant, + source_location: unresolved_reference.source_location, + ) + end + + fully_qualified_references + end end - end - sig do - params( - constant_name_inspectors: T::Array[ConstantNameInspector], - root_node: ::AST::Node, - root_path: String, - ).void - end - def initialize( - constant_name_inspectors:, - root_node:, - root_path: - ) - @constant_name_inspectors = constant_name_inspectors - @root_path = root_path - @local_constant_definitions = ParsedConstantDefinitions.new(root_node: root_node) - end + sig do + params( + constant_name_inspectors: T::Array[ConstantNameInspector], + root_node: ::AST::Node, + root_path: String, + ).void + end + def initialize( + constant_name_inspectors:, + root_node:, + root_path: + ) + @constant_name_inspectors = constant_name_inspectors + @root_path = root_path + @local_constant_definitions = ParsedConstantDefinitions.new(root_node: root_node) + end - sig do - params( - node: Parser::AST::Node, - ancestors: T::Array[Parser::AST::Node], - relative_file: String - ).returns(T.nilable(UnresolvedReference)) - end - def reference_from_node(node, ancestors:, relative_file:) - constant_name = T.let(nil, T.nilable(String)) + sig do + params( + node: Parser::AST::Node, + ancestors: T::Array[Parser::AST::Node], + relative_file: String + ).returns(T.nilable(UnresolvedReference)) + end + def reference_from_node(node, ancestors:, relative_file:) + constant_name = T.let(nil, T.nilable(String)) - @constant_name_inspectors.each do |inspector| - constant_name = inspector.constant_name_from_node(node, ancestors: ancestors) + @constant_name_inspectors.each do |inspector| + constant_name = inspector.constant_name_from_node(node, ancestors: ancestors) - break if constant_name - end + break if constant_name + end - if constant_name - reference_from_constant( - constant_name, - node: node, - ancestors: ancestors, - relative_file: relative_file - ) + if constant_name + reference_from_constant( + constant_name, + node: node, + ancestors: ancestors, + relative_file: relative_file + ) + end end - end - private + private - sig do - params( - constant_name: String, - node: Parser::AST::Node, - ancestors: T::Array[Parser::AST::Node], - relative_file: String - ).returns(T.nilable(UnresolvedReference)) - end - def reference_from_constant(constant_name, node:, ancestors:, relative_file:) - namespace_path = NodeHelpers.enclosing_namespace_path(node, ancestors: ancestors) + sig do + params( + constant_name: String, + node: Parser::AST::Node, + ancestors: T::Array[Parser::AST::Node], + relative_file: String + ).returns(T.nilable(UnresolvedReference)) + end + def reference_from_constant(constant_name, node:, ancestors:, relative_file:) + namespace_path = NodeHelpers.enclosing_namespace_path(node, ancestors: ancestors) - return if local_reference?(constant_name, NodeHelpers.name_location(node), namespace_path) + return if local_reference?(constant_name, NodeHelpers.name_location(node), namespace_path) - location = NodeHelpers.location(node) + location = NodeHelpers.location(node) - UnresolvedReference.new( - constant_name: constant_name, - namespace_path: namespace_path, - relative_path: relative_file, - source_location: location - ) - end + UnresolvedReference.new( + constant_name: constant_name, + namespace_path: namespace_path, + relative_path: relative_file, + source_location: location + ) + end - def local_reference?(constant_name, name_location, namespace_path) - @local_constant_definitions.local_reference?( - constant_name, - location: name_location, - namespace_path: namespace_path - ) + def local_reference?(constant_name, name_location, namespace_path) + @local_constant_definitions.local_reference?( + constant_name, + location: name_location, + namespace_path: namespace_path + ) + end end end end diff --git a/lib/packwerk/private/run_context.rb b/lib/packwerk/private/run_context.rb index f04db76f5..06189e35c 100644 --- a/lib/packwerk/private/run_context.rb +++ b/lib/packwerk/private/run_context.rb @@ -4,132 +4,134 @@ require "constant_resolver" module Packwerk - # Holds the context of a Packwerk run across multiple files. - class RunContext - extend T::Sig - - class << self + module Private + # Holds the context of a Packwerk run across multiple files. + class RunContext extend T::Sig + class << self + extend T::Sig + + sig do + params(configuration: Configuration).returns(RunContext) + end + def from_configuration(configuration) + inflector = ActiveSupport::Inflector + + new( + root_path: configuration.root_path, + load_paths: configuration.load_paths, + package_paths: configuration.package_paths, + inflector: inflector, + custom_associations: configuration.custom_associations, + cache_enabled: configuration.cache_enabled?, + cache_directory: configuration.cache_directory, + config_path: configuration.config_path, + ) + end + end + sig do - params(configuration: Configuration).returns(RunContext) + params( + root_path: String, + load_paths: T::Hash[String, Module], + inflector: T.class_of(ActiveSupport::Inflector), + cache_directory: Pathname, + config_path: T.nilable(String), + package_paths: T.nilable(T.any(T::Array[String], String)), + custom_associations: AssociationInspector::CustomAssociations, + checkers: T::Array[Checker], + cache_enabled: T::Boolean, + ).void end - def from_configuration(configuration) - inflector = ActiveSupport::Inflector - - new( - root_path: configuration.root_path, - load_paths: configuration.load_paths, - package_paths: configuration.package_paths, - inflector: inflector, - custom_associations: configuration.custom_associations, - cache_enabled: configuration.cache_enabled?, - cache_directory: configuration.cache_directory, - config_path: configuration.config_path, + def initialize( + root_path:, + load_paths:, + inflector:, + cache_directory:, + config_path: nil, + package_paths: nil, + custom_associations: [], + checkers: Checker.all, + cache_enabled: false + ) + @root_path = root_path + @load_paths = load_paths + @package_paths = package_paths + @inflector = inflector + @custom_associations = custom_associations + @checkers = checkers + @cache_enabled = cache_enabled + @cache_directory = cache_directory + @config_path = config_path + + @file_processor = T.let(nil, T.nilable(FileProcessor)) + @context_provider = T.let(nil, T.nilable(ConstantDiscovery)) + @package_set = T.let(nil, T.nilable(PackageSet)) + # We need to initialize this before we fork the process, see https://github.com/Shopify/packwerk/issues/182 + @cache = T.let( + Cache.new(enable_cache: @cache_enabled, cache_directory: @cache_directory, config_path: @config_path), Cache ) end - end - sig do - params( - root_path: String, - load_paths: T::Hash[String, Module], - inflector: T.class_of(ActiveSupport::Inflector), - cache_directory: Pathname, - config_path: T.nilable(String), - package_paths: T.nilable(T.any(T::Array[String], String)), - custom_associations: AssociationInspector::CustomAssociations, - checkers: T::Array[Checker], - cache_enabled: T::Boolean, - ).void - end - def initialize( - root_path:, - load_paths:, - inflector:, - cache_directory:, - config_path: nil, - package_paths: nil, - custom_associations: [], - checkers: Checker.all, - cache_enabled: false - ) - @root_path = root_path - @load_paths = load_paths - @package_paths = package_paths - @inflector = inflector - @custom_associations = custom_associations - @checkers = checkers - @cache_enabled = cache_enabled - @cache_directory = cache_directory - @config_path = config_path - - @file_processor = T.let(nil, T.nilable(FileProcessor)) - @context_provider = T.let(nil, T.nilable(ConstantDiscovery)) - @package_set = T.let(nil, T.nilable(PackageSet)) - # We need to initialize this before we fork the process, see https://github.com/Shopify/packwerk/issues/182 - @cache = T.let( - Cache.new(enable_cache: @cache_enabled, cache_directory: @cache_directory, config_path: @config_path), Cache - ) - end + sig { params(relative_file: String).returns(T::Array[Packwerk::Offense]) } + def process_file(relative_file:) + processed_file = file_processor.call(relative_file) - sig { params(relative_file: String).returns(T::Array[Packwerk::Offense]) } - def process_file(relative_file:) - processed_file = file_processor.call(relative_file) - - references = ReferenceExtractor.get_fully_qualified_references_from( - processed_file.unresolved_references, - context_provider - ) - reference_checker = ReferenceChecking::ReferenceChecker.new(@checkers) + references = ReferenceExtractor.get_fully_qualified_references_from( + processed_file.unresolved_references, + context_provider + ) + reference_checker = ReferenceChecking::ReferenceChecker.new(@checkers) - processed_file.offenses + references.flat_map { |reference| reference_checker.call(reference) } - end + processed_file.offenses + references.flat_map { |reference| reference_checker.call(reference) } + end - sig { returns(PackageSet) } - def package_set - @package_set ||= ::Packwerk::PackageSet.load_all_from(@root_path, package_pathspec: @package_paths) - end + sig { returns(PackageSet) } + def package_set + @package_set ||= ::Packwerk::PackageSet.load_all_from(@root_path, package_pathspec: @package_paths) + end - private + private - sig { returns(FileProcessor) } - def file_processor - @file_processor ||= FileProcessor.new(node_processor_factory: node_processor_factory, cache: @cache) - end + sig { returns(FileProcessor) } + def file_processor + @file_processor ||= FileProcessor.new(node_processor_factory: node_processor_factory, cache: @cache) + end - sig { returns(NodeProcessorFactory) } - def node_processor_factory - NodeProcessorFactory.new( - context_provider: context_provider, - root_path: @root_path, - constant_name_inspectors: constant_name_inspectors - ) - end + sig { returns(NodeProcessorFactory) } + def node_processor_factory + NodeProcessorFactory.new( + context_provider: context_provider, + root_path: @root_path, + constant_name_inspectors: constant_name_inspectors + ) + end - sig { returns(ConstantDiscovery) } - def context_provider - @context_provider ||= ConstantDiscovery.new( - constant_resolver: resolver, - packages: package_set - ) - end + sig { returns(ConstantDiscovery) } + def context_provider + @context_provider ||= ConstantDiscovery.new( + constant_resolver: resolver, + packages: package_set + ) + end - sig { returns(ConstantResolver) } - def resolver - ConstantResolver.new( - root_path: @root_path, - load_paths: @load_paths, - inflector: @inflector, - ) - end + sig { returns(ConstantResolver) } + def resolver + ConstantResolver.new( + root_path: @root_path, + load_paths: @load_paths, + inflector: @inflector, + ) + end - sig { returns(T::Array[ConstantNameInspector]) } - def constant_name_inspectors - [ - ConstNodeInspector.new, - AssociationInspector.new(inflector: @inflector, custom_associations: @custom_associations), - ] + sig { returns(T::Array[ConstantNameInspector]) } + def constant_name_inspectors + [ + ConstNodeInspector.new, + AssociationInspector.new(inflector: @inflector, custom_associations: @custom_associations), + ] + end end end end diff --git a/lib/packwerk/private/unresolved_reference.rb b/lib/packwerk/private/unresolved_reference.rb index 3dd99a660..85de0fcd4 100644 --- a/lib/packwerk/private/unresolved_reference.rb +++ b/lib/packwerk/private/unresolved_reference.rb @@ -2,15 +2,17 @@ # frozen_string_literal: true module Packwerk - # An unresolved reference from a file in one package to a constant that may be defined in a different package. - # Unresolved means that we know how it's referred to in the file, - # and we have enough context on that reference to figure out the fully qualified reference such that we - # can produce a Reference in a separate pass. However, we have not yet resolved it to its fully qualified version. - UnresolvedReference = Struct.new( - :constant_name, - :namespace_path, - :relative_path, - :source_location, - keyword_init: true, - ) + module Private + # An unresolved reference from a file in one package to a constant that may be defined in a different package. + # Unresolved means that we know how it's referred to in the file, + # and we have enough context on that reference to figure out the fully qualified reference such that we + # can produce a Reference in a separate pass. However, we have not yet resolved it to its fully qualified version. + UnresolvedReference = Struct.new( + :constant_name, + :namespace_path, + :relative_path, + :source_location, + keyword_init: true, + ) + end end diff --git a/lib/packwerk/reference_offense.rb b/lib/packwerk/reference_offense.rb index 2f267fc7d..ac1825e94 100644 --- a/lib/packwerk/reference_offense.rb +++ b/lib/packwerk/reference_offense.rb @@ -18,7 +18,7 @@ class ReferenceOffense < Offense reference: Packwerk::Reference, violation_type: String, message: String, - location: T.nilable(Node::Location) + location: T.nilable(Private::Node::Location) ) .void end diff --git a/sorbet/rbi/shims/packwerk/private/unresolved_reference.rbi b/sorbet/rbi/shims/packwerk/private/unresolved_reference.rbi new file mode 100644 index 000000000..1c943ec22 --- /dev/null +++ b/sorbet/rbi/shims/packwerk/private/unresolved_reference.rbi @@ -0,0 +1,35 @@ +# typed: true + +module Packwerk + module Private + class UnresolvedReference + sig do + params( + constant_name: String, + namespace_path: T.nilable(T::Array[String]), + relative_path: String, + source_location: T.nilable(Private::Node::Location), + ).void + end + def initialize( + constant_name:, + namespace_path:, + relative_path:, + source_location: + ) + end + + sig { returns(String) } + attr_reader(:constant_name) + + sig { returns(T.nilable(T::Array[String])) } + attr_reader(:namespace_path) + + sig { returns(String) } + attr_reader(:relative_path) + + sig { returns(T.nilable(Private::Node::Location)) } + attr_reader(:source_location) + end + end +end diff --git a/sorbet/rbi/shims/packwerk/reference.rbi b/sorbet/rbi/shims/packwerk/reference.rbi index ea78d6885..4518d14b9 100644 --- a/sorbet/rbi/shims/packwerk/reference.rbi +++ b/sorbet/rbi/shims/packwerk/reference.rbi @@ -6,8 +6,8 @@ module Packwerk params( package: Package, relative_path: String, - constant: ConstantDiscovery::ConstantContext, - source_location: T.nilable(Node::Location), + constant: Private::ConstantDiscovery::ConstantContext, + source_location: T.nilable(Private::Node::Location), ).void end def initialize( @@ -24,10 +24,10 @@ module Packwerk sig { returns(T.nilable(String)) } attr_reader(:relative_path) - sig { returns(ConstantDiscovery::ConstantContext) } + sig { returns(Private::ConstantDiscovery::ConstantContext) } attr_reader(:constant) - sig { returns(T.nilable(Node::Location)) } + sig { returns(T.nilable(Private::Node::Location)) } attr_reader(:source_location) end end diff --git a/sorbet/rbi/shims/packwerk/unresolved_reference.rbi b/sorbet/rbi/shims/packwerk/unresolved_reference.rbi deleted file mode 100644 index 7ec46046b..000000000 --- a/sorbet/rbi/shims/packwerk/unresolved_reference.rbi +++ /dev/null @@ -1,33 +0,0 @@ -# typed: true - -module Packwerk - class UnresolvedReference - sig do - params( - constant_name: String, - namespace_path: T.nilable(T::Array[String]), - relative_path: String, - source_location: T.nilable(Node::Location), - ).void - end - def initialize( - constant_name:, - namespace_path:, - relative_path:, - source_location: - ) - end - - sig { returns(String) } - attr_reader(:constant_name) - - sig { returns(T.nilable(T::Array[String])) } - attr_reader(:namespace_path) - - sig { returns(String) } - attr_reader(:relative_path) - - sig { returns(T.nilable(Node::Location)) } - attr_reader(:source_location) - end -end diff --git a/test/const_node_inspector_test.rb b/test/const_node_inspector_test.rb index 5a642f970..9f1ad9b1f 100644 --- a/test/const_node_inspector_test.rb +++ b/test/const_node_inspector_test.rb @@ -7,7 +7,7 @@ module Packwerk class ConstNodeInspectorTest < ActiveSupport::TestCase setup do - @inspector = ConstNodeInspector.new + @inspector = Private::ConstNodeInspector.new end test "#constant_name_from_node should ignore any non-const nodes" do @@ -44,7 +44,7 @@ class ConstNodeInspectorTest < ActiveSupport::TestCase test "#constant_name_from_node should return correct name for simple class definition" do parent = parse("class Order; end") - node = NodeHelpers.each_child(parent).entries[0] + node = Private::NodeHelpers.each_child(parent).entries[0] constant_name = @inspector.constant_name_from_node(node, ancestors: [parent]) @@ -53,8 +53,8 @@ class ConstNodeInspectorTest < ActiveSupport::TestCase test "#constant_name_from_node should return correct name for nested and compact class definition" do grandparent = parse("module Foo::Bar; class Sales::Order; end; end") - parent = NodeHelpers.each_child(grandparent).entries[1] # module node; second child is the body of the module - node = NodeHelpers.each_child(parent).entries[0] # class node; first child is constant + parent = Private::NodeHelpers.each_child(grandparent).entries[1] # module node; second child is the body of the module + node = Private::NodeHelpers.each_child(parent).entries[0] # class node; first child is constant constant_name = @inspector.constant_name_from_node(node, ancestors: [parent, grandparent]) @@ -63,8 +63,8 @@ class ConstNodeInspectorTest < ActiveSupport::TestCase test "#constant_name_from_node should gracefully return nil for dynamically namespaced constants" do grandparent = parse("module CsvExportSharedTests; setup do self.class::HEADERS end; end") - parent = NodeHelpers.each_child(grandparent).entries[1] # setup do self.class::HEADERS end - node = NodeHelpers.each_child(parent).entries[2] # self.class::HEADERS + parent = Private::NodeHelpers.each_child(grandparent).entries[1] # setup do self.class::HEADERS end + node = Private::NodeHelpers.each_child(parent).entries[2] # self.class::HEADERS constant_name = @inspector.constant_name_from_node(node, ancestors: [parent, grandparent]) diff --git a/test/node_helpers_test.rb b/test/node_helpers_test.rb index 6b76253fa..46df23f9b 100644 --- a/test/node_helpers_test.rb +++ b/test/node_helpers_test.rb @@ -7,41 +7,41 @@ require "parser" module Packwerk - class NodeHelpersTest < ActiveSupport::TestCase + class Private::NodeHelpersTest < ActiveSupport::TestCase test ".class_or_module_name returns the name of a class being defined with the class keyword" do node = parse("class My::Class; end") - assert_equal "My::Class", NodeHelpers.class_or_module_name(node) + assert_equal "My::Class", Private::NodeHelpers.class_or_module_name(node) end test ".class_or_module_name returns the name of a module being defined with the module keyword" do node = parse("module My::Module; end") - assert_equal "My::Module", NodeHelpers.class_or_module_name(node) + assert_equal "My::Module", Private::NodeHelpers.class_or_module_name(node) end test ".constant_name returns the name of a constant being referenced" do node = parse("My::Constant") - assert_equal "My::Constant", NodeHelpers.constant_name(node) + assert_equal "My::Constant", Private::NodeHelpers.constant_name(node) end test ".constant_name returns the name of a constant being assigned to" do node = parse("My::Constant = 42") - assert_equal "My::Constant", NodeHelpers.constant_name(node) + assert_equal "My::Constant", Private::NodeHelpers.constant_name(node) end test ".constant_name raises a TypeError for dynamically namespaced constants" do node = parse("self.class::HEADERS") - assert_raises(NodeHelpers::TypeError) { NodeHelpers.constant_name(node) } + assert_raises(Private::NodeHelpers::TypeError) { Private::NodeHelpers.constant_name(node) } end test ".constant_name preserves the name of a fully-qualified constant" do node = parse("::My::Constant") - assert_equal "::My::Constant", NodeHelpers.constant_name(node) + assert_equal "::My::Constant", Private::NodeHelpers.constant_name(node) end test ".each_child with a block iterates over all child nodes" do node = parse("My::Constant = 6 * 7") children = [] - NodeHelpers.each_child(node) do |child| + Private::NodeHelpers.each_child(node) do |child| children << child end assert_equal [parse("My"), parse("6 * 7")], children @@ -49,7 +49,7 @@ class NodeHelpersTest < ActiveSupport::TestCase test ".each_child without a block returns an enumerator" do node = parse("My::Constant = 6 * 7") - children = NodeHelpers.each_child(node) + children = Private::NodeHelpers.each_child(node) assert_instance_of Enumerator, children assert_equal [parse("My"), parse("6 * 7")], children.entries end @@ -57,63 +57,63 @@ class NodeHelpersTest < ActiveSupport::TestCase test "#enclosing_namespace_path should return empty path for const node" do node = parse("Order") - path = NodeHelpers.enclosing_namespace_path(node, ancestors: []) + path = Private::NodeHelpers.enclosing_namespace_path(node, ancestors: []) assert_equal [], path end test "#enclosing_namespace_path should return correct path for simple class definition" do parent = parse("class Order; end") - node = NodeHelpers.each_child(parent).entries[0] + node = Private::NodeHelpers.each_child(parent).entries[0] - path = NodeHelpers.enclosing_namespace_path(node, ancestors: [parent]) + path = Private::NodeHelpers.enclosing_namespace_path(node, ancestors: [parent]) assert_equal ["Order"], path end test "#enclosing_namespace_path should skip child class name when finding path for parent class" do grandparent = parse("module Sales; class Order < Base; end; end") - parent = NodeHelpers.each_child(grandparent).entries[1] # module node; second child is the body of the module - node = NodeHelpers.each_child(parent).entries[1] # class node; second child is parent + parent = Private::NodeHelpers.each_child(grandparent).entries[1] # module node; second child is the body of the module + node = Private::NodeHelpers.each_child(parent).entries[1] # class node; second child is parent - path = NodeHelpers.enclosing_namespace_path(node, ancestors: [parent, grandparent]) + path = Private::NodeHelpers.enclosing_namespace_path(node, ancestors: [parent, grandparent]) assert_equal ["Sales"], path end test "#enclosing_namespace_path should return correct path for nested and compact class definition" do grandparent = parse("module Foo::Bar; class Sales::Order; end; end") - parent = NodeHelpers.each_child(grandparent).entries[1] # module node; second child is the body of the module - node = NodeHelpers.each_child(parent).entries[0] # class node; first child is constant + parent = Private::NodeHelpers.each_child(grandparent).entries[1] # module node; second child is the body of the module + node = Private::NodeHelpers.each_child(parent).entries[0] # class node; first child is constant - path = NodeHelpers.enclosing_namespace_path(node, ancestors: [parent, grandparent]) + path = Private::NodeHelpers.enclosing_namespace_path(node, ancestors: [parent, grandparent]) assert_equal ["Foo::Bar", "Sales::Order"], path end test ".literal_value returns the value of a string node" do node = parse("'Hello'") - assert_equal "Hello", NodeHelpers.literal_value(node) + assert_equal "Hello", Private::NodeHelpers.literal_value(node) end test ".literal_value returns the value of a symbol node" do node = parse(":world") - assert_equal :world, NodeHelpers.literal_value(node) + assert_equal :world, Private::NodeHelpers.literal_value(node) end test ".location returns a source location" do node = parse("HELLO = 'World'") - assert_kind_of Node::Location, NodeHelpers.location(node) + assert_kind_of Private::Node::Location, Private::NodeHelpers.location(node) end test ".method_arguments returns the arguments of a method call" do node = parse("a.b(:c, 'd', E)") - assert_equal [parse(":c"), parse("'d'"), parse("E")], NodeHelpers.method_arguments(node) + assert_equal [parse(":c"), parse("'d'"), parse("E")], Private::NodeHelpers.method_arguments(node) end test ".method_name returns the name of a method call" do node = parse("a.b(:c, 'd', E)") - assert_equal :b, NodeHelpers.method_name(node) + assert_equal :b, Private::NodeHelpers.method_name(node) end test ".module_name_from_definition returns the name of the class being defined" do @@ -125,7 +125,7 @@ class NodeHelpersTest < ActiveSupport::TestCase ["My::Class = Class.new do end", "My::Class"], ].each do |class_definition, name| node = parse(class_definition) - assert_equal name, NodeHelpers.module_name_from_definition(node) + assert_equal name, Private::NodeHelpers.module_name_from_definition(node) end end @@ -138,7 +138,7 @@ class NodeHelpersTest < ActiveSupport::TestCase ["My::Module = Module.new do end", "My::Module"], ].each do |module_definition, name| node = parse(module_definition) - assert_equal name, NodeHelpers.module_name_from_definition(node) + assert_equal name, Private::NodeHelpers.module_name_from_definition(node) end end @@ -154,94 +154,94 @@ class NodeHelpersTest < ActiveSupport::TestCase "MyConstant = -> {}", ].each do |module_definition| node = parse(module_definition) - assert_nil NodeHelpers.module_name_from_definition(node) + assert_nil Private::NodeHelpers.module_name_from_definition(node) end end test ".name_location returns a source location for a constant" do node = parse("HELLO") - assert_kind_of Node::Location, NodeHelpers.name_location(node) + assert_kind_of Private::Node::Location, Private::NodeHelpers.name_location(node) end test ".name_location returns a source location for a constant assignment" do node = parse("HELLO = 'World'") - assert_kind_of Node::Location, NodeHelpers.name_location(node) + assert_kind_of Private::Node::Location, Private::NodeHelpers.name_location(node) end test ".name_location returns nil for a method call" do node = parse("has_many :hellos") - assert_nil NodeHelpers.name_location(node) + assert_nil Private::NodeHelpers.name_location(node) end test ".parent_class returns the constant referring to the parent class in a class being defined with the class keyword" do node = parse("class B < A; end") - assert_equal parse("A"), NodeHelpers.parent_class(node) + assert_equal parse("A"), Private::NodeHelpers.parent_class(node) end test ".parent_module_name returns the name of a constant’s enclosing module" do grandparent = parse("module A; class B; C; end end") - parent = NodeHelpers.each_child(grandparent).entries[1] # "class B; C; end" - assert_equal "A::B", NodeHelpers.parent_module_name(ancestors: [parent, grandparent]) + parent = Private::NodeHelpers.each_child(grandparent).entries[1] # "class B; C; end" + assert_equal "A::B", Private::NodeHelpers.parent_module_name(ancestors: [parent, grandparent]) end test ".parent_module_name returns Object if the constant has no enclosing module" do - assert_equal "Object", NodeHelpers.parent_module_name(ancestors: []) + assert_equal "Object", Private::NodeHelpers.parent_module_name(ancestors: []) end test ".parent_module_name supports constant assignment" do grandparent = parse("module A; B = Class.new do C end end") - parent = NodeHelpers.each_child(grandparent).entries[1] # "B = Class.new do C end" - assert_equal "A::B", NodeHelpers.parent_module_name(ancestors: [parent, grandparent]) + parent = Private::NodeHelpers.each_child(grandparent).entries[1] # "B = Class.new do C end" + assert_equal "A::B", Private::NodeHelpers.parent_module_name(ancestors: [parent, grandparent]) end test ".parent_module_name supports class_eval with no receiver" do grandparent = parse("module A; class_eval do C; end end") - parent = NodeHelpers.each_child(grandparent).entries[1] # "class_eval do C; end" - assert_equal "A", NodeHelpers.parent_module_name(ancestors: [parent, grandparent]) + parent = Private::NodeHelpers.each_child(grandparent).entries[1] # "class_eval do C; end" + assert_equal "A", Private::NodeHelpers.parent_module_name(ancestors: [parent, grandparent]) end test ".parent_module_name supports class_eval with an explicit receiver" do grandparent = parse("module A; B.class_eval do C; end end") - parent = NodeHelpers.each_child(grandparent).entries[1] # "B.class_eval do C; end" - assert_equal "A::B", NodeHelpers.parent_module_name(ancestors: [parent, grandparent]) + parent = Private::NodeHelpers.each_child(grandparent).entries[1] # "B.class_eval do C; end" + assert_equal "A::B", Private::NodeHelpers.parent_module_name(ancestors: [parent, grandparent]) end test ".class? can identify a class node" do - assert NodeHelpers.class?(parse("class Fruit; end")) + assert Private::NodeHelpers.class?(parse("class Fruit; end")) end test ".constant? can identify a constant node" do - assert NodeHelpers.constant?(parse("Oranges")) + assert Private::NodeHelpers.constant?(parse("Oranges")) end test ".constant_assignment? can identify a constant assignment node" do - assert NodeHelpers.constant_assignment?(parse("Apples = 13")) + assert Private::NodeHelpers.constant_assignment?(parse("Apples = 13")) end test ".hash? can identify a hash node" do - assert NodeHelpers.hash?(parse("{ pears: 3, bananas: 6 }")) + assert Private::NodeHelpers.hash?(parse("{ pears: 3, bananas: 6 }")) end test ".method_call? can identify a method call node" do - assert NodeHelpers.method_call?(parse("quantity(bananas)")) + assert Private::NodeHelpers.method_call?(parse("quantity(bananas)")) end test ".string? can identify a string node" do - assert NodeHelpers.string?(parse("'cashew apple'")) + assert Private::NodeHelpers.string?(parse("'cashew apple'")) end test ".symbol? can identify a symbol node" do - assert NodeHelpers.symbol?(parse(":papaya")) + assert Private::NodeHelpers.symbol?(parse(":papaya")) end test ".value_from_hash looks up the node for a key in a hash" do hash_node = parse("{ apples: 13, oranges: 27 }") - assert_equal parse("13"), NodeHelpers.value_from_hash(hash_node, :apples) + assert_equal parse("13"), Private::NodeHelpers.value_from_hash(hash_node, :apples) end test ".value_from_hash returns nil if a key isn't found in a hash" do hash_node = parse("{ apples: 13, oranges: 27 }") - assert_nil NodeHelpers.value_from_hash(hash_node, :pears) + assert_nil Private::NodeHelpers.value_from_hash(hash_node, :pears) end private diff --git a/test/support/factory_helper.rb b/test/support/factory_helper.rb index cf50332f8..d6f5d2e23 100644 --- a/test/support/factory_helper.rb +++ b/test/support/factory_helper.rb @@ -8,9 +8,9 @@ def build_reference( destination_package: Packwerk::Package.new(name: "components/destination", config: {}), path: "some/path.rb", constant_name: "::SomeName", - source_location: Node::Location.new(2, 12) + source_location: Private::Node::Location.new(2, 12) ) - constant = ConstantDiscovery::ConstantContext.new( + constant = Private::ConstantDiscovery::ConstantContext.new( constant_name, "some/location.rb", destination_package, diff --git a/test/unit/application_load_paths_test.rb b/test/unit/application_load_paths_test.rb index 810bf3f22..87dfb0d4c 100644 --- a/test/unit/application_load_paths_test.rb +++ b/test/unit/application_load_paths_test.rb @@ -10,7 +10,7 @@ class ApplicationLoadPathsTest < Minitest::Test rails_root = Pathname.new("/application/") relative_path = "app/models" absolute_path = rails_root.join(relative_path) - relative_path_strings = ApplicationLoadPaths.relative_path_strings( + relative_path_strings = Private::ApplicationLoadPaths.relative_path_strings( { absolute_path => Object }, rails_root: rails_root ) @@ -22,7 +22,7 @@ class ApplicationLoadPathsTest < Minitest::Test 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( + filtered_paths = Private::ApplicationLoadPaths.filter_relevant_paths( paths, bundle_path: Pathname.new("/application/vendor/"), rails_root: Pathname.new("/application/") @@ -35,7 +35,7 @@ class ApplicationLoadPathsTest < Minitest::Test 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( + filtered_paths = Private::ApplicationLoadPaths.filter_relevant_paths( paths, bundle_path: Pathname.new("/application/vendor/"), rails_root: Pathname.new("/application/") @@ -45,10 +45,10 @@ class ApplicationLoadPathsTest < Minitest::Test 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(:require_application).with("/application", "test").once.returns(true) + Private::ApplicationLoadPaths.expects(:filter_relevant_paths).once.returns(Pathname.new("/fake_path").to_s => Object) + Private::ApplicationLoadPaths.expects(:require_application).with("/application", "test").once.returns(true) - ApplicationLoadPaths.extract_relevant_paths("/application", "test") + Private::ApplicationLoadPaths.extract_relevant_paths("/application", "test") end end end diff --git a/test/unit/application_validator_test.rb b/test/unit/application_validator_test.rb index 998d4bf9e..b2a43f74c 100644 --- a/test/unit/application_validator_test.rb +++ b/test/unit/application_validator_test.rb @@ -73,9 +73,9 @@ class ApplicationValidatorTest < Minitest::Test private - sig { returns(ApplicationValidator) } + sig { returns(Private::ApplicationValidator) } def validator - @application_validator ||= ApplicationValidator.new + @application_validator ||= Private::ApplicationValidator.new end end end diff --git a/test/unit/association_inspector_test.rb b/test/unit/association_inspector_test.rb index ffbea088d..ad151aa60 100644 --- a/test/unit/association_inspector_test.rb +++ b/test/unit/association_inspector_test.rb @@ -5,56 +5,56 @@ require "parser_test_helper" module Packwerk - class AssociationInspectorTest < Minitest::Test + class Private::AssociationInspectorTest < Minitest::Test setup do @inflector = ActiveSupport::Inflector end test "#association? understands custom associations" do node = parse("has_lots :order") - inspector = AssociationInspector.new(inflector: @inflector, custom_associations: [:has_lots]) + inspector = Private::AssociationInspector.new(inflector: @inflector, custom_associations: [:has_lots]) assert_equal "Order", inspector.constant_name_from_node(node, ancestors: []) end test "finds target constant for simple association" do node = parse("has_one :order") - inspector = AssociationInspector.new(inflector: @inflector) + inspector = Private::AssociationInspector.new(inflector: @inflector) assert_equal "Order", inspector.constant_name_from_node(node, ancestors: []) end test "finds target constant for association that pluralizes" do node = parse("has_many :orders") - inspector = AssociationInspector.new(inflector: @inflector) + inspector = Private::AssociationInspector.new(inflector: @inflector) assert_equal "Order", inspector.constant_name_from_node(node, ancestors: []) end test "finds target constant for association if explicitly specified" do node = parse("has_one :cool_order, { class_name: 'Order' }") - inspector = AssociationInspector.new(inflector: @inflector) + inspector = Private::AssociationInspector.new(inflector: @inflector) assert_equal "Order", inspector.constant_name_from_node(node, ancestors: []) end test "rejects method calls that are not associations" do node = parse('puts "Hello World"') - inspector = AssociationInspector.new(inflector: @inflector) + inspector = Private::AssociationInspector.new(inflector: @inflector) assert_nil inspector.constant_name_from_node(node, ancestors: []) end test "gives up on metaprogrammed associations" do node = parse("has_one association_name") - inspector = AssociationInspector.new(inflector: @inflector) + inspector = Private::AssociationInspector.new(inflector: @inflector) assert_nil inspector.constant_name_from_node(node, ancestors: []) end test "gives up on dynamic class name" do node = parse("has_one :order, class_name: Order.name") - inspector = AssociationInspector.new(inflector: @inflector) + inspector = Private::AssociationInspector.new(inflector: @inflector) assert_nil inspector.constant_name_from_node(node, ancestors: []) end diff --git a/test/unit/cache_test.rb b/test/unit/cache_test.rb index 8cc4bbe85..756c78551 100644 --- a/test/unit/cache_test.rb +++ b/test/unit/cache_test.rb @@ -17,7 +17,7 @@ class CacheTest < Minitest::Test end teardown do - Cache.new( + Private::Cache.new( enable_cache: true, config_path: "packwerk.yml", cache_directory: Pathname.new("tmp/cache/packwerk") @@ -31,19 +31,19 @@ class CacheTest < Minitest::Test filepath.write("class MyClass; end") unresolved_references = [ - UnresolvedReference.new( + Private::UnresolvedReference.new( constant_name: "MyConstant", namespace_path: [], relative_path: "path/of_exile.rb", - source_location: Node::Location.new(5, 5) + source_location: Private::Node::Location.new(5, 5) ), ] - FileProcessor.any_instance.stubs(:references_from_ast).returns(unresolved_references) + Private::FileProcessor.any_instance.stubs(:references_from_ast).returns(unresolved_references) configuration = Configuration.from_path configuration.stubs(cache_enabled?: true) - parse_run = ParseRun.new(relative_file_set: Set.new([filepath.to_s]), configuration: configuration) + parse_run = Private::ParseRun.new(relative_file_set: Set.new([filepath.to_s]), configuration: configuration) parse_run.update_todo parse_run.update_todo @@ -51,7 +51,7 @@ class CacheTest < Minitest::Test assert_equal cache_files.count, 3 digest_file = Pathname.new("tmp/cache/packwerk").join(Digest::MD5.hexdigest(filepath.to_s)) - cached_result = Cache::CacheContents.deserialize(digest_file.read) + cached_result = Private::Cache::CacheContents.deserialize(digest_file.read) assert_equal cached_result.unresolved_references, unresolved_references end end diff --git a/test/unit/cli_test.rb b/test/unit/cli_test.rb index 9fc35faf6..da87aa0c6 100644 --- a/test/unit/cli_test.rb +++ b/test/unit/cli_test.rb @@ -28,14 +28,14 @@ class CliTest < Minitest::Test configuration = Configuration.new({ "parallel" => false }) configuration.stubs(load_paths: {}) - RunContext.any_instance.stubs(:process_file).at_least_once.returns([offense]) + Private::RunContext.any_instance.stubs(:process_file).at_least_once.returns([offense]) string_io = StringIO.new cli = ::Packwerk::Cli.new(out: string_io, configuration: configuration) # TODO: Dependency injection for a "target finder" (https://github.com/Shopify/packwerk/issues/164) - FilesForProcessing.any_instance.stubs( + Private::FilesForProcessing.any_instance.stubs( files: Set.new([file_path]) ) @@ -58,7 +58,7 @@ class CliTest < Minitest::Test configuration = Configuration.new({ "parallel" => false }) configuration.stubs(load_paths: {}) - RunContext.any_instance.stubs(:process_file) + Private::RunContext.any_instance.stubs(:process_file) .at_least(2) .returns([offense]) .raises(Interrupt) @@ -68,7 +68,7 @@ class CliTest < Minitest::Test cli = ::Packwerk::Cli.new(out: string_io, configuration: configuration) - FilesForProcessing.any_instance.stubs( + Private::FilesForProcessing.any_instance.stubs( files: Set.new([file_path, "test.rb", "foo.rb"]) ) @@ -96,7 +96,7 @@ class CliTest < Minitest::Test cli = ::Packwerk::Cli.new(out: string_io) validator = typed_mock(check_all: Validator::Result.new(ok: true)) - ApplicationValidator.expects(:new).returns(validator) + Private::ApplicationValidator.expects(:new).returns(validator) success = cli.execute_command(["validate"]) @@ -110,7 +110,7 @@ class CliTest < Minitest::Test cli = ::Packwerk::Cli.new(out: string_io) validator = typed_mock(check_all: Validator::Result.new(ok: false, error_value: "I'm an error")) - ApplicationValidator.expects(:new).returns(validator) + Private::ApplicationValidator.expects(:new).returns(validator) success = cli.execute_command(["validate"]) @@ -162,7 +162,7 @@ def show_strict_mode_violations(_offenses) configuration = Configuration.new configuration.stubs(load_paths: {}) - RunContext.any_instance.stubs(:process_file) + Private::RunContext.any_instance.stubs(:process_file) .returns([offense]) string_io = StringIO.new @@ -173,7 +173,7 @@ def show_strict_mode_violations(_offenses) offenses_formatter: T.unsafe(offenses_formatter).new ) - FilesForProcessing.any_instance.stubs( + Private::FilesForProcessing.any_instance.stubs( files: Set.new([file_path]) ) @@ -193,7 +193,7 @@ def show_strict_mode_violations(_offenses) violation_message = "This is a violation of code health." offense = Offense.new(file: file_path, message: violation_message) - RunContext.any_instance.stubs(:process_file) + Private::RunContext.any_instance.stubs(:process_file) .returns([offense]) cli = T.let(nil, T.nilable(Packwerk::Cli)) @@ -205,11 +205,11 @@ def show_strict_mode_violations(_offenses) end reset_formatters - ExtensionLoader.stub(:require, mock_require_method) do + Private::ExtensionLoader.stub(:require, mock_require_method) do cli = ::Packwerk::Cli.new(out: string_io) end - FilesForProcessing.any_instance.stubs( + Private::FilesForProcessing.any_instance.stubs( files: Set.new([file_path]) ) @@ -231,7 +231,7 @@ def show_strict_mode_violations(_offenses) violation_message = "This is a violation of code health." offense = Offense.new(file: file_path, message: violation_message) - RunContext.any_instance.stubs(:process_file) + Private::RunContext.any_instance.stubs(:process_file) .returns([offense]) cli = T.let(nil, T.nilable(Packwerk::Cli)) @@ -243,11 +243,11 @@ def show_strict_mode_violations(_offenses) end reset_formatters - ExtensionLoader.stub(:require, mock_require_method) do + Private::ExtensionLoader.stub(:require, mock_require_method) do cli = ::Packwerk::Cli.new(out: string_io) end - FilesForProcessing.any_instance.stubs( + Private::FilesForProcessing.any_instance.stubs( files: Set.new([file_path]) ) diff --git a/test/unit/configuration_test.rb b/test/unit/configuration_test.rb index 5cd5cb38a..57d8eb8d9 100644 --- a/test/unit/configuration_test.rb +++ b/test/unit/configuration_test.rb @@ -72,7 +72,7 @@ class ConfigurationTest < Minitest::Test require required_thing end - ExtensionLoader.stub(:require, mock_require_method) do + Private::ExtensionLoader.stub(:require, mock_require_method) do Configuration.from_path end assert defined?(MyLocalExtension) @@ -87,7 +87,7 @@ class ConfigurationTest < Minitest::Test mock_require_method = ->(required_thing) do required_things << required_thing end - ExtensionLoader.stub(:require, mock_require_method) do + Private::ExtensionLoader.stub(:require, mock_require_method) do Configuration.from_path end diff --git a/test/unit/constant_discovery_test.rb b/test/unit/constant_discovery_test.rb index 7e77a3bf7..fed56c1d5 100644 --- a/test/unit/constant_discovery_test.rb +++ b/test/unit/constant_discovery_test.rb @@ -19,7 +19,7 @@ def setup load_paths["components/sales/app/internal"] = Business - @discovery = ConstantDiscovery.new( + @discovery = Private::ConstantDiscovery.new( constant_resolver: ConstantResolver.new(root_path: @root_path, load_paths: load_paths), packages: PackageSet.load_all_from(@root_path) ) @@ -43,7 +43,7 @@ def setup test "raises with helpful message if there is a constant resolver error" do constant_resolver = typed_mock constant_resolver.stubs(:resolve).raises(ConstantResolver::Error, "initial error message") - discovery = ConstantDiscovery.new( + discovery = Private::ConstantDiscovery.new( constant_resolver: constant_resolver, packages: PackageSet.load_all_from(@root_path) ) diff --git a/test/unit/file_processor_test.rb b/test/unit/file_processor_test.rb index 931a0bb7d..b8ce0193a 100644 --- a/test/unit/file_processor_test.rb +++ b/test/unit/file_processor_test.rb @@ -11,12 +11,12 @@ class FileProcessorTest < Minitest::Test setup do @node_processor_factory = typed_mock @node_processor = typed_mock - @cache = Cache.new( + @cache = Private::Cache.new( enable_cache: false, cache_directory: Pathname.new("tmp/cache/packwerk"), config_path: "packwerk.yml" ) - @file_processor = FileProcessor.new(node_processor_factory: @node_processor_factory, cache: @cache) + @file_processor = Private::FileProcessor.new(node_processor_factory: @node_processor_factory, cache: @cache) end test "#call visits all nodes in a file path with no references" do @@ -32,11 +32,11 @@ class FileProcessorTest < Minitest::Test end test "#call visits a node in file path with an reference" do - unresolved_reference = UnresolvedReference.new( + unresolved_reference = Private::UnresolvedReference.new( constant_name: "SomeName", namespace_path: [], relative_path: "tempfile", - source_location: Node::Location.new(3, 22), + source_location: Private::Node::Location.new(3, 22), ) @node_processor_factory.expects(:for).returns(@node_processor) @node_processor.expects(:call).returns(unresolved_reference) @@ -60,25 +60,25 @@ class FileProcessorTest < Minitest::Test reference = typed_mock @node_processor_factory.expects(:for).returns(@node_processor) @node_processor.expects(:call).with do |node, ancestors| - NodeHelpers.class?(node) && # class Hello; world; end - NodeHelpers.class_or_module_name(node) == "Hello" && + Private::NodeHelpers.class?(node) && # class Hello; world; end + Private::NodeHelpers.class_or_module_name(node) == "Hello" && ancestors.empty? end.returns(nil) @node_processor.expects(:call).with do |node, ancestors| parent = ancestors.first # class Hello; world; end - NodeHelpers.constant?(node) && # Hello - NodeHelpers.constant_name(node) == "Hello" && + Private::NodeHelpers.constant?(node) && # Hello + Private::NodeHelpers.constant_name(node) == "Hello" && ancestors.length == 1 && - NodeHelpers.class?(parent) && - NodeHelpers.class_or_module_name(parent) == "Hello" + Private::NodeHelpers.class?(parent) && + Private::NodeHelpers.class_or_module_name(parent) == "Hello" end.returns(nil) @node_processor.expects(:call).with do |node, ancestors| parent = ancestors.first # class Hello; world; end - NodeHelpers.method_call?(node) && # world - NodeHelpers.method_name(node) == :world && + Private::NodeHelpers.method_call?(node) && # world + Private::NodeHelpers.method_name(node) == :world && ancestors.length == 1 && - NodeHelpers.class?(parent) && - NodeHelpers.class_or_module_name(parent) == "Hello" + Private::NodeHelpers.class?(parent) && + Private::NodeHelpers.class_or_module_name(parent) == "Hello" end.returns(reference) processed_file = tempfile(name: "hello_world", content: "class Hello; world; end") do |file_path| @@ -100,9 +100,9 @@ class FileProcessorTest < Minitest::Test test "#call with an invalid syntax doesn't parse node" do @node_processor_factory.expects(:for).never - file_processor = FileProcessor.new( + file_processor = Private::FileProcessor.new( node_processor_factory: @node_processor_factory, - cache: Cache.new( + cache: Private::Cache.new( enable_cache: false, cache_directory: Pathname.new("tmp/cache/packwerk"), config_path: "packwerk.yml" diff --git a/test/unit/files_for_processing_test.rb b/test/unit/files_for_processing_test.rb index 8972d4ef7..be3390244 100644 --- a/test/unit/files_for_processing_test.rb +++ b/test/unit/files_for_processing_test.rb @@ -4,7 +4,7 @@ require "test_helper" module Packwerk - class FilesForProcessingTest < Minitest::Test + class Private::FilesForProcessingTest < Minitest::Test include ApplicationFixtureHelper setup do @@ -19,7 +19,7 @@ class FilesForProcessingTest < Minitest::Test end test "fetch with custom paths includes only include glob in custom paths" do - files = FilesForProcessing.fetch( + files = Private::FilesForProcessing.fetch( relative_file_paths: [@package_path], configuration: @configuration, ).files @@ -28,7 +28,7 @@ class FilesForProcessingTest < Minitest::Test end test "fetch with custom paths excludes the exclude glob in custom paths" do - files = FilesForProcessing.fetch( + files = Private::FilesForProcessing.fetch( relative_file_paths: [@package_path], configuration: @configuration ).files @@ -38,25 +38,25 @@ class FilesForProcessingTest < Minitest::Test end test "fetch with no custom paths includes only include glob across codebase" do - files = FilesForProcessing.fetch(relative_file_paths: [], configuration: @configuration).files + files = Private::FilesForProcessing.fetch(relative_file_paths: [], configuration: @configuration).files assert_all_match(files, @configuration.include) end test "fetch with no custom paths excludes the exclude glob across codebase" do - files = FilesForProcessing.fetch(relative_file_paths: [], configuration: @configuration).files + files = Private::FilesForProcessing.fetch(relative_file_paths: [], configuration: @configuration).files excluded_file_patterns = @configuration.exclude.map { |pattern| File.join(@configuration.root_path, pattern) } refute_any_match(files, Set.new(excluded_file_patterns)) end test "fetch does not return duplicated file paths" do - files = FilesForProcessing.fetch(relative_file_paths: [], configuration: @configuration).files + files = Private::FilesForProcessing.fetch(relative_file_paths: [], configuration: @configuration).files assert_equal files, Set.new(files) end test "fetch with custom paths without ignoring nested packages includes only include glob in custom paths including nested package files" do - files = FilesForProcessing.fetch( + files = Private::FilesForProcessing.fetch( relative_file_paths: ["."], configuration: @configuration, ignore_nested_packages: false @@ -66,7 +66,7 @@ class FilesForProcessingTest < Minitest::Test end test "fetch with no custom paths ignoring nested packages includes only include glob across codebase" do - files = FilesForProcessing.fetch( + files = Private::FilesForProcessing.fetch( relative_file_paths: [], configuration: @configuration, ignore_nested_packages: true @@ -76,7 +76,7 @@ class FilesForProcessingTest < Minitest::Test end test "fetch with custom paths ignoring nested packages includes only include glob in custom paths without nested package files" do - files = FilesForProcessing.fetch( + files = Private::FilesForProcessing.fetch( relative_file_paths: ["."], configuration: @configuration, ignore_nested_packages: true @@ -87,7 +87,7 @@ class FilesForProcessingTest < Minitest::Test end test "fetch with custom paths for files includes only include glob in custom paths" do - files = FilesForProcessing.fetch( + files = Private::FilesForProcessing.fetch( relative_file_paths: [ "components/sales/app/models/order.rb", "components/sales/app/views/order.html.erb", diff --git a/test/unit/node_visitor_test.rb b/test/unit/node_visitor_test.rb index 637fa8de5..19ac6047f 100644 --- a/test/unit/node_visitor_test.rb +++ b/test/unit/node_visitor_test.rb @@ -11,7 +11,7 @@ class NodeVisitorTest < Minitest::Test test "#visit visits the correct number of nodes" do node_processor = typed_mock node_processor.expects(:call).times(3).returns(["an offense"]) - file_node_visitor = NodeVisitor.new(node_processor: node_processor) + file_node_visitor = Private::NodeVisitor.new(node_processor: node_processor) node = ParserTestHelper.parse("class Hello; world; end") result = [] diff --git a/test/unit/offense_test.rb b/test/unit/offense_test.rb index ad2be702d..1a218315f 100644 --- a/test/unit/offense_test.rb +++ b/test/unit/offense_test.rb @@ -6,7 +6,7 @@ module Packwerk class OffenseTest < Minitest::Test setup do - location = Node::Location.new(90, 10) + location = Private::Node::Location.new(90, 10) file = "components/platform/shop.rb" message = "Violation of developer rights" @offense = Offense.new(location: location, file: file, message: message) diff --git a/test/unit/parse_run_test.rb b/test/unit/parse_run_test.rb index 391498fc6..1a3b01a36 100644 --- a/test/unit/parse_run_test.rb +++ b/test/unit/parse_run_test.rb @@ -5,7 +5,7 @@ require "rails_test_helper" module Packwerk - class ParseRunTest < Minitest::Test + class Private::ParseRunTest < Minitest::Test include FactoryHelper include ApplicationFixtureHelper @@ -19,10 +19,10 @@ class ParseRunTest < Minitest::Test test "#update-todo returns success when there are no offenses" do use_template(:minimal) - RunContext.any_instance.stubs(:process_file).returns([]) + Private::RunContext.any_instance.stubs(:process_file).returns([]) OffenseCollection.any_instance.expects(:dump_package_todo_files).once - parse_run = ParseRun.new( + parse_run = Private::ParseRun.new( relative_file_set: Set.new(["path/of/exile.rb"]), configuration: Configuration.from_path ) @@ -38,10 +38,10 @@ class ParseRunTest < Minitest::Test test "#update-todo returns exit code 1 when there are offenses" do use_template(:minimal) offense = Offense.new(file: "path/of/exile.rb", message: "something") - RunContext.any_instance.stubs(:process_file).returns([offense]) + Private::RunContext.any_instance.stubs(:process_file).returns([offense]) OffenseCollection.any_instance.expects(:dump_package_todo_files).once - parse_run = ParseRun.new( + parse_run = Private::ParseRun.new( relative_file_set: Set.new(["path/of/exile.rb"]), configuration: Configuration.from_path ) @@ -62,7 +62,7 @@ class ParseRunTest < Minitest::Test test "#update-todo returns exit code 1 when ran with file args" do use_template(:minimal) - parse_run = ParseRun.new( + parse_run = Private::ParseRun.new( relative_file_set: Set.new(["path/of/exile.rb"]), file_set_specified: true, configuration: Configuration.from_path @@ -106,7 +106,7 @@ def something - a/b/c.rb YML - parse_run = ParseRun.new( + parse_run = Private::ParseRun.new( relative_file_set: Set.new(["app/models/my_model.rb", "components/sales/app/models/order.rb"]), configuration: Configuration.from_path ) @@ -133,12 +133,12 @@ def something PackageTodo.any_instance.stubs(:listed?).returns(true) out = StringIO.new - parse_run = ParseRun.new( + parse_run = Private::ParseRun.new( relative_file_set: Set.new(["some/path.rb"]), configuration: Configuration.new({ "parallel" => false }), progress_formatter: Formatters::ProgressFormatter.new(out) ) - RunContext.any_instance.stubs(:process_file).returns([offense]) + Private::RunContext.any_instance.stubs(:process_file).returns([offense]) result = parse_run.check expected_output = <<~EOS @@ -210,12 +210,12 @@ def something ) out = StringIO.new - parse_run = ParseRun.new( + parse_run = Private::ParseRun.new( relative_file_set: Set.new([file_to_check]), configuration: Configuration.new({ "parallel" => false }), progress_formatter: Formatters::ProgressFormatter.new(out) ) - RunContext.any_instance.stubs(:process_file).returns([offense1, offense2]) + Private::RunContext.any_instance.stubs(:process_file).returns([offense1, offense2]) result = parse_run.check expected_output = <<~EOS @@ -256,12 +256,12 @@ def something YML out = StringIO.new - parse_run = ParseRun.new( + parse_run = Private::ParseRun.new( relative_file_set: Set.new([file_to_check]), configuration: Configuration.new({ "parallel" => false }), progress_formatter: Formatters::ProgressFormatter.new(out) ) - RunContext.any_instance.stubs(:process_file).returns([]) + Private::RunContext.any_instance.stubs(:process_file).returns([]) result = parse_run.check expected_output = <<~EOS @@ -303,7 +303,7 @@ def something YML out = StringIO.new - parse_run = ParseRun.new( + parse_run = Private::ParseRun.new( relative_file_set: Set.new([file_to_check]), configuration: Configuration.new({ "parallel" => false }), progress_formatter: Formatters::ProgressFormatter.new(out) @@ -315,7 +315,7 @@ def something violation_type: ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE ) - RunContext.any_instance.stubs(:process_file).returns([offense]) + Private::RunContext.any_instance.stubs(:process_file).returns([offense]) result = parse_run.check expected_output = <<~EOS @@ -350,7 +350,7 @@ def something YML out = StringIO.new - parse_run = ParseRun.new( + parse_run = Private::ParseRun.new( relative_file_set: Set.new(["components/source/some/path.rb"]), configuration: Configuration.new({ "parallel" => false }), progress_formatter: Formatters::ProgressFormatter.new(out) @@ -362,7 +362,7 @@ def something violation_type: ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE ) - RunContext.any_instance.stubs(:process_file).returns([offense]) + Private::RunContext.any_instance.stubs(:process_file).returns([offense]) result = parse_run.check expected_output = <<~EOS @@ -406,7 +406,7 @@ def something out = StringIO.new - parse_run = ParseRun.new( + parse_run = Private::ParseRun.new( relative_file_set: Set.new([file_to_check]), configuration: Configuration.new({ "parallel" => false, "offenses_formatter" => "plain" }), progress_formatter: Formatters::ProgressFormatter.new(out) @@ -418,7 +418,7 @@ def something violation_type: ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE ) - RunContext.any_instance.stubs(:process_file).returns([offense]) + Private::RunContext.any_instance.stubs(:process_file).returns([offense]) result = parse_run.check expected_output = <<~EOS @@ -454,12 +454,12 @@ def something PackageTodo.any_instance.stubs(:listed?).returns(true) OffenseCollection.any_instance.stubs(:stale_violations?).returns(true) out = StringIO.new - parse_run = ParseRun.new( + parse_run = Private::ParseRun.new( relative_file_set: Set.new(["some/path.rb"]), configuration: Configuration.new({ "parallel" => false }), progress_formatter: Formatters::ProgressFormatter.new(out) ) - RunContext.any_instance.stubs(:process_file).returns([offense]) + Private::RunContext.any_instance.stubs(:process_file).returns([offense]) result = parse_run.check expected_output = <<~EOS @@ -491,11 +491,11 @@ def something message: "some message", violation_type: ReferenceChecking::Checkers::DependencyChecker::VIOLATION_TYPE ) - parse_run = ParseRun.new( + parse_run = Private::ParseRun.new( relative_file_set: Set.new(["some/path.rb", "some/other_path.rb"]), configuration: Configuration.new ) - RunContext.any_instance.stubs(:process_file).returns([offense]).returns([offense2]) + Private::RunContext.any_instance.stubs(:process_file).returns([offense]).returns([offense2]) result = parse_run.check refute result.status diff --git a/test/unit/parsed_constant_definitions_test.rb b/test/unit/parsed_constant_definitions_test.rb index 93e5086a6..1c5540b4b 100644 --- a/test/unit/parsed_constant_definitions_test.rb +++ b/test/unit/parsed_constant_definitions_test.rb @@ -5,9 +5,9 @@ require "parser_test_helper" module Packwerk - class ParsedConstantDefinitionsTest < Minitest::Test + class Private::ParsedConstantDefinitionsTest < Minitest::Test test "recognizes constant assignment" do - definitions = ParsedConstantDefinitions.new( + definitions = Private::ParsedConstantDefinitions.new( root_node: parse_code('HELLO = "World"') ) @@ -15,7 +15,7 @@ class ParsedConstantDefinitionsTest < Minitest::Test end test "recognizes class or module definitions" do - definitions = ParsedConstantDefinitions.new( + definitions = Private::ParsedConstantDefinitions.new( root_node: parse_code("module Sales; class Order; end; end") ) @@ -24,7 +24,7 @@ class ParsedConstantDefinitionsTest < Minitest::Test end test "recognizes constants that are more fully qualified" do - definitions = ParsedConstantDefinitions.new( + definitions = Private::ParsedConstantDefinitions.new( root_node: parse_code('module Sales; HELLO = "World"; end') ) @@ -34,7 +34,7 @@ class ParsedConstantDefinitionsTest < Minitest::Test end test "understands fully qualified references" do - definitions = ParsedConstantDefinitions.new( + definitions = Private::ParsedConstantDefinitions.new( root_node: parse_code("module Sales; class Order; end; end") ) @@ -45,7 +45,7 @@ class ParsedConstantDefinitionsTest < Minitest::Test end test "recognizes compact nested constant definition" do - definitions = ParsedConstantDefinitions.new( + definitions = Private::ParsedConstantDefinitions.new( root_node: parse_code("module Sales::Order::Something; end") ) @@ -56,7 +56,7 @@ class ParsedConstantDefinitionsTest < Minitest::Test end test "recognizes compact nested constant assignment" do - definitions = ParsedConstantDefinitions.new( + definitions = Private::ParsedConstantDefinitions.new( root_node: parse_code('Sales::HELLO = "World"') ) @@ -67,7 +67,7 @@ class ParsedConstantDefinitionsTest < Minitest::Test end test "recognizes compact nested constant definition and assignment" do - definitions = ParsedConstantDefinitions.new( + definitions = Private::ParsedConstantDefinitions.new( root_node: parse_code('module Sales::Order; Something::HELLO = "World"; end') ) @@ -85,7 +85,7 @@ class ParsedConstantDefinitionsTest < Minitest::Test end test "recognizes local constant reference from sub-namespace" do - definitions = ParsedConstantDefinitions.new( + definitions = Private::ParsedConstantDefinitions.new( root_node: parse_code("module Something; class Else; HELLO = 1; end; end") ) @@ -93,7 +93,7 @@ class ParsedConstantDefinitionsTest < Minitest::Test end test "recognizes multiple constants nested in a shared ancestor module" do - definitions = ParsedConstantDefinitions.new( + definitions = Private::ParsedConstantDefinitions.new( root_node: parse_code("module Sales; class Order; end; class Thing; end; end") ) @@ -104,21 +104,21 @@ class ParsedConstantDefinitionsTest < Minitest::Test test "doesn't count definition as reference" do ast = parse_code("class HelloWorld; end") - const_node = NodeHelpers.each_child(ast).find { |n| NodeHelpers.constant?(n) } + const_node = Private::NodeHelpers.each_child(ast).find { |n| Private::NodeHelpers.constant?(n) } - definitions = ParsedConstantDefinitions.new( + definitions = Private::ParsedConstantDefinitions.new( root_node: ast ) - assert definitions.local_reference?(NodeHelpers.constant_name(const_node)) - refute definitions.local_reference?(NodeHelpers.constant_name(const_node), - location: NodeHelpers.name_location(const_node)) + assert definitions.local_reference?(Private::NodeHelpers.constant_name(const_node)) + refute definitions.local_reference?(Private::NodeHelpers.constant_name(const_node), + location: Private::NodeHelpers.name_location(const_node)) end test "handles empty files" do ast = parse_code("# just a comment") - definitions = ParsedConstantDefinitions.new( + definitions = Private::ParsedConstantDefinitions.new( root_node: ast ) @@ -127,14 +127,14 @@ class ParsedConstantDefinitionsTest < Minitest::Test test ".reference_qualifications generates all possible qualifications for a reference" do qualifications = - ParsedConstantDefinitions.reference_qualifications("Order", namespace_path: ["Sales", "Internal"]) + Private::ParsedConstantDefinitions.reference_qualifications("Order", namespace_path: ["Sales", "Internal"]) assert_equal ["::Order", "::Sales::Order", "::Sales::Internal::Order"].sort, qualifications.sort end test ".reference_qualifications generates all possible qualifications for a reference even when there are nil nodes in the namespace path" do qualifications = - ParsedConstantDefinitions.reference_qualifications("Order", namespace_path: [nil, "Sales", "Internal"]) + Private::ParsedConstantDefinitions.reference_qualifications("Order", namespace_path: [nil, "Sales", "Internal"]) assert_equal ["::Order", "::Sales::Order", "::Sales::Internal::Order"].sort, qualifications.sort end diff --git a/test/unit/reference_extractor_test.rb b/test/unit/reference_extractor_test.rb index 20bcbcff2..4b7494de3 100644 --- a/test/unit/reference_extractor_test.rb +++ b/test/unit/reference_extractor_test.rb @@ -20,7 +20,7 @@ def setup resolver = ConstantResolver.new(root_path: app_dir, load_paths: load_paths) packages = ::Packwerk::PackageSet.load_all_from(app_dir) - @context_provider = ConstantDiscovery.new( + @context_provider = Private::ConstantDiscovery.new( constant_resolver: resolver, packages: packages ) @@ -162,7 +162,7 @@ def teardown test "passes all arguments to association inspector" do call = "has_many :clowns, class_name: 'Order'" - arguments = NodeHelpers.method_arguments(ParserTestHelper.parse(call)) + arguments = Private::NodeHelpers.method_arguments(ParserTestHelper.parse(call)) process( "class Entry; #{call}; end", "components/timeline/app/models/entry.rb", @@ -214,7 +214,7 @@ def teardown private class DummyAssociationInspector - include ConstantNameInspector + include Private::ConstantNameInspector def initialize(association: false, reference_name: "Dummy", expected_args: nil) @association = association @@ -224,9 +224,9 @@ def initialize(association: false, reference_name: "Dummy", expected_args: nil) def constant_name_from_node(node, ancestors:) return nil unless @association - return nil unless NodeHelpers.method_call?(node) + return nil unless Private::NodeHelpers.method_call?(node) - args = NodeHelpers.method_arguments(node) + args = Private::NodeHelpers.method_arguments(node) if @expected_args && @expected_args != args raise("expected arguments don't match.\nExpected:\n#{@expected_args}\nActual:\n#{args}") end @@ -235,13 +235,13 @@ def constant_name_from_node(node, ancestors:) end end - DEFAULT_INSPECTORS = [ConstNodeInspector.new, DummyAssociationInspector.new] + DEFAULT_INSPECTORS = [Private::ConstNodeInspector.new, DummyAssociationInspector.new] def process(code, file_path, constant_name_inspectors = DEFAULT_INSPECTORS) root_node = ParserTestHelper.parse(code) file_path = to_app_path(file_path) - extractor = ReferenceExtractor.new( + extractor = Private::ReferenceExtractor.new( constant_name_inspectors: constant_name_inspectors, root_node: root_node, root_path: app_dir @@ -254,7 +254,7 @@ def process(code, file_path, constant_name_inspectors = DEFAULT_INSPECTORS) file_path: Pathname.new(file_path).relative_path_from(app_dir).to_s ) - ReferenceExtractor.get_fully_qualified_references_from( + Private::ReferenceExtractor.get_fully_qualified_references_from( unresolved_references, @context_provider ) @@ -263,7 +263,7 @@ def process(code, file_path, constant_name_inspectors = DEFAULT_INSPECTORS) def find_references_in_ast(root_node, ancestors:, extractor:, file_path:) references = [extractor.reference_from_node(root_node, ancestors: ancestors, relative_file: file_path)] - child_references = NodeHelpers.each_child(root_node).flat_map do |child| + child_references = Private::NodeHelpers.each_child(root_node).flat_map do |child| find_references_in_ast(child, ancestors: [root_node] + ancestors, extractor: extractor, file_path: file_path) end From f6990351aef50c2c965b883bc043d27dadc2a93f Mon Sep 17 00:00:00 2001 From: Alex Evanczuk Date: Mon, 12 Dec 2022 09:59:03 -0500 Subject: [PATCH 16/16] rubocop --- test/const_node_inspector_test.rb | 6 ++++-- test/node_helpers_test.rb | 14 +++++++++----- test/unit/application_load_paths_test.rb | 4 +++- test/unit/association_inspector_test.rb | 2 +- test/unit/files_for_processing_test.rb | 2 +- test/unit/parse_run_test.rb | 2 +- test/unit/parsed_constant_definitions_test.rb | 2 +- 7 files changed, 20 insertions(+), 12 deletions(-) diff --git a/test/const_node_inspector_test.rb b/test/const_node_inspector_test.rb index 9f1ad9b1f..27d2f0c0e 100644 --- a/test/const_node_inspector_test.rb +++ b/test/const_node_inspector_test.rb @@ -53,8 +53,10 @@ class ConstNodeInspectorTest < ActiveSupport::TestCase test "#constant_name_from_node should return correct name for nested and compact class definition" do grandparent = parse("module Foo::Bar; class Sales::Order; end; end") - parent = Private::NodeHelpers.each_child(grandparent).entries[1] # module node; second child is the body of the module - node = Private::NodeHelpers.each_child(parent).entries[0] # class node; first child is constant + # module node; second child is the body of the module + parent = Private::NodeHelpers.each_child(grandparent).entries[1] + # class node; first child is constant + node = Private::NodeHelpers.each_child(parent).entries[0] constant_name = @inspector.constant_name_from_node(node, ancestors: [parent, grandparent]) diff --git a/test/node_helpers_test.rb b/test/node_helpers_test.rb index 46df23f9b..f4698deaa 100644 --- a/test/node_helpers_test.rb +++ b/test/node_helpers_test.rb @@ -7,7 +7,7 @@ require "parser" module Packwerk - class Private::NodeHelpersTest < ActiveSupport::TestCase + class NodeHelpersTest < ActiveSupport::TestCase test ".class_or_module_name returns the name of a class being defined with the class keyword" do node = parse("class My::Class; end") assert_equal "My::Class", Private::NodeHelpers.class_or_module_name(node) @@ -73,8 +73,10 @@ class Private::NodeHelpersTest < ActiveSupport::TestCase test "#enclosing_namespace_path should skip child class name when finding path for parent class" do grandparent = parse("module Sales; class Order < Base; end; end") - parent = Private::NodeHelpers.each_child(grandparent).entries[1] # module node; second child is the body of the module - node = Private::NodeHelpers.each_child(parent).entries[1] # class node; second child is parent + # module node; second child is the body of the module + parent = Private::NodeHelpers.each_child(grandparent).entries[1] + # class node; second child is parent + node = Private::NodeHelpers.each_child(parent).entries[1] path = Private::NodeHelpers.enclosing_namespace_path(node, ancestors: [parent, grandparent]) @@ -83,8 +85,10 @@ class Private::NodeHelpersTest < ActiveSupport::TestCase test "#enclosing_namespace_path should return correct path for nested and compact class definition" do grandparent = parse("module Foo::Bar; class Sales::Order; end; end") - parent = Private::NodeHelpers.each_child(grandparent).entries[1] # module node; second child is the body of the module - node = Private::NodeHelpers.each_child(parent).entries[0] # class node; first child is constant + # module node; second child is the body of the module + parent = Private::NodeHelpers.each_child(grandparent).entries[1] + # class node; first child is constant + node = Private::NodeHelpers.each_child(parent).entries[0] path = Private::NodeHelpers.enclosing_namespace_path(node, ancestors: [parent, grandparent]) diff --git a/test/unit/application_load_paths_test.rb b/test/unit/application_load_paths_test.rb index 87dfb0d4c..0b838d30e 100644 --- a/test/unit/application_load_paths_test.rb +++ b/test/unit/application_load_paths_test.rb @@ -45,7 +45,9 @@ class ApplicationLoadPathsTest < Minitest::Test end test ".extract_relevant_paths calls out to filter the paths" do - Private::ApplicationLoadPaths.expects(:filter_relevant_paths).once.returns(Pathname.new("/fake_path").to_s => Object) + Private::ApplicationLoadPaths.expects(:filter_relevant_paths).once.returns( + Pathname.new("/fake_path").to_s => Object + ) Private::ApplicationLoadPaths.expects(:require_application).with("/application", "test").once.returns(true) Private::ApplicationLoadPaths.extract_relevant_paths("/application", "test") diff --git a/test/unit/association_inspector_test.rb b/test/unit/association_inspector_test.rb index ad151aa60..3ab8e53d8 100644 --- a/test/unit/association_inspector_test.rb +++ b/test/unit/association_inspector_test.rb @@ -5,7 +5,7 @@ require "parser_test_helper" module Packwerk - class Private::AssociationInspectorTest < Minitest::Test + class AssociationInspectorTest < Minitest::Test setup do @inflector = ActiveSupport::Inflector end diff --git a/test/unit/files_for_processing_test.rb b/test/unit/files_for_processing_test.rb index be3390244..a6cf4be1b 100644 --- a/test/unit/files_for_processing_test.rb +++ b/test/unit/files_for_processing_test.rb @@ -4,7 +4,7 @@ require "test_helper" module Packwerk - class Private::FilesForProcessingTest < Minitest::Test + class FilesForProcessingTest < Minitest::Test include ApplicationFixtureHelper setup do diff --git a/test/unit/parse_run_test.rb b/test/unit/parse_run_test.rb index 1a3b01a36..a75d9d149 100644 --- a/test/unit/parse_run_test.rb +++ b/test/unit/parse_run_test.rb @@ -5,7 +5,7 @@ require "rails_test_helper" module Packwerk - class Private::ParseRunTest < Minitest::Test + class ParseRunTest < Minitest::Test include FactoryHelper include ApplicationFixtureHelper diff --git a/test/unit/parsed_constant_definitions_test.rb b/test/unit/parsed_constant_definitions_test.rb index 1c5540b4b..cb81fbee7 100644 --- a/test/unit/parsed_constant_definitions_test.rb +++ b/test/unit/parsed_constant_definitions_test.rb @@ -5,7 +5,7 @@ require "parser_test_helper" module Packwerk - class Private::ParsedConstantDefinitionsTest < Minitest::Test + class ParsedConstantDefinitionsTest < Minitest::Test test "recognizes constant assignment" do definitions = Private::ParsedConstantDefinitions.new( root_node: parse_code('HELLO = "World"')