Pull load paths from application

USAGE.md

@@ -68,7 +68,6 @@ Packwerk reads from the `packwerk.yml` configuration file in the root directory.
 | include              | **/*.{rb,rake,erb}                        | list of patterns for folder paths to include |
 | exclude              | {bin,node_modules,script,tmp,vendor}/**/* | list of patterns for folder paths to exclude |
 | package_paths        | **/                                       | a single pattern or a list of patterns to find package configuration files, see: [Defining packages](#Defining-packages) |
-| load_paths           | All application autoload paths            | list of load paths |
 | custom_associations  | N/A                                       | list of custom associations, if any |
 | parallel             | true                                      | when true, fork code parsing out to subprocesses |
 

lib/packwerk/application_load_paths.rb

@@ -18,6 +18,15 @@ def extract_relevant_paths(root, environment)
         relative_path_strings(relevant_paths)
       end
 
+      sig { params(root: String).void }
+      def self.validate_rails_app!(root)
+        environment_file = "#{root}/config/environment"
+
+        unless File.file?("#{environment_file}.rb")
+          raise "A Rails application could not be found in #{root}"
+        end
+      end
+
       sig { returns(T::Array[String]) }
       def extract_application_autoload_paths
         Rails.application.railties

lib/packwerk/application_validator.rb

@@ -14,15 +14,12 @@ def initialize(config_file_path:, configuration:, environment:)
       @config_file_path = config_file_path
       @configuration = configuration
       @environment = environment
-
-      @application_load_paths = ApplicationLoadPaths.extract_relevant_paths(configuration.root_path, environment)
     end
 
     Result = Struct.new(:ok?, :error_value)
 
     def check_all
       results = [
-        check_autoload_path_cache,
         check_package_manifests_for_privacy,
         check_package_manifest_syntax,
         check_application_structure,
@@ -36,19 +33,10 @@ def check_all
       merge_results(results)
     end
 
-    def check_autoload_path_cache
-      expected = Set.new(@application_load_paths)
-      actual = Set.new(@configuration.load_paths)
-      if expected == actual
-        Result.new(true)
-      else
-        Result.new(
-          false,
-          "Load path cache in #{@config_file_path} incorrect!\n"\
-          "Paths missing from file:\n#{format_yaml_strings(expected - actual)}\n"\
-          "Extraneous load paths in file:\n#{format_yaml_strings(actual - expected)}"
-        )
-      end
+    def validate_rails_app!
+      # Let's make sure we don't generate any exceptions when trying to fetch the load paths for the Rails application
+      # This will raise if the application is not a valid Rails application
+      @configuration.load_paths
     end
 
     def check_package_manifests_for_privacy

lib/packwerk/cli.rb

@@ -87,11 +87,13 @@ def init
     end
 
     def generate_configs
+      checker.validate_rails_app!
+
       configuration_file = Packwerk::Generators::ConfigurationFile.generate(
-        load_paths: Packwerk::ApplicationLoadPaths.extract_relevant_paths(@configuration.root_path, @environment),
         root: @configuration.root_path,
         out: @out
       )
+
       inflections_file = Packwerk::Generators::InflectionsFile.generate(root: @configuration.root_path, out: @out)
       root_package = Packwerk::Generators::RootPackage.generate(root: @configuration.root_path, out: @out)
 
@@ -139,11 +141,6 @@ def fetch_files_to_process(paths, ignore_nested_packages)
 
     def validate(_paths)
       @progress_formatter.started_validation do
-        checker = Packwerk::ApplicationValidator.new(
-          config_file_path: @configuration.config_path,
-          configuration: @configuration,
-          environment: @environment,
-        )
         result = checker.check_all
 
         list_validation_errors(result)
@@ -152,6 +149,14 @@ def validate(_paths)
       end
     end
 
+    def checker
+      Packwerk::ApplicationValidator.new(
+        config_file_path: @configuration.config_path,
+        configuration: @configuration,
+        environment: @environment,
+      )
+    end
+
     def list_validation_errors(result)
       @out.puts
       if result.ok?

lib/packwerk/configuration.rb

@@ -34,24 +34,35 @@ def from_packwerk_config(path)
     DEFAULT_EXCLUDE_GLOBS = ["{bin,node_modules,script,tmp,vendor}/**/*"]
 
     attr_reader(
-      :include, :exclude, :root_path, :package_paths, :custom_associations, :load_paths, :inflections_file,
+      :include, :exclude, :root_path, :package_paths, :custom_associations, :inflections_file,
       :config_path,
     )
 
     def initialize(configs = {}, config_path: nil)
+      if configs["load_paths"]
+        warning = <<~WARNING
+          DEPRECATION WARNING: The 'load_paths' key in `packwerk.yml` is deprecated.
+          This value is no longer cached, and you can remove the key from `packwerk.yml`.
+        WARNING
+
+        warn(warning)
+      end
       @include = configs["include"] || DEFAULT_INCLUDE_GLOBS
       @exclude = configs["exclude"] || DEFAULT_EXCLUDE_GLOBS
       root = config_path ? File.dirname(config_path) : "."
       @root_path = File.expand_path(root)
       @package_paths = configs["package_paths"] || "**/"
       @custom_associations = configs["custom_associations"] || []
-      @load_paths = (configs["load_paths"] || []).uniq
       @inflections_file = File.expand_path(configs["inflections_file"] || "config/inflections.yml", @root_path)
       @parallel = configs.key?("parallel") ? configs["parallel"] : true
 
       @config_path = config_path
     end
 
+    def load_paths
+      @load_paths ||= ApplicationLoadPaths.extract_relevant_paths(@root_path, "test")
+    end
+
     def parallel?
       @parallel
     end

lib/packwerk/constant_discovery.rb

@@ -45,7 +45,7 @@ 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 + "\n Make sure autoload paths are added to the config file.")
+        raise(ConstantResolver::Error, e.message)
       end
 
       return unless constant

lib/packwerk/generators/configuration_file.rb

@@ -11,18 +11,15 @@ class ConfigurationFile
       CONFIGURATION_TEMPLATE_FILE_PATH = "templates/packwerk.yml.erb"
 
       class << self
-        def generate(load_paths:, root:, out:)
-          new(load_paths: load_paths, root: root, out: out).generate
+        def generate(root:, out:)
+          new(root: root, out: out).generate
         end
       end
 
-      sig { params(load_paths: T::Array[String], root: String, out: T.any(StringIO, IO)).void }
-      def initialize(load_paths:, root:, out: $stdout)
-        @load_paths = load_paths
+      sig { params(root: String, out: T.any(StringIO, IO)).void }
+      def initialize(root:, out: $stdout)
         @root = root
         @out = out
-
-        set_template_variables
       end
 
       sig { returns(T::Boolean) }
@@ -43,18 +40,6 @@ def generate
 
       private
 
-      def set_template_variables
-        @load_paths_formatted = if @load_paths.empty?
-          "# load_paths:\n# - 'app/models'\n"
-        else
-          @load_paths.map { |path| "- #{path}\n" }.join
-        end
-
-        @load_paths_comment = unless @load_paths.empty?
-          "# These load paths were auto generated by Packwerk.\nload_paths:\n"
-        end
-      end
-
       def render
         ERB.new(template, trim_mode: "-").result(binding)
       end

lib/packwerk/generators/templates/packwerk.yml.erb

@@ -12,9 +12,6 @@
 # Patterns to find package configuration files
 # package_paths: "**/"
 
-# List of application load paths
-<%= @load_paths_comment -%>
-<%= @load_paths_formatted %>
 # List of custom associations, if any
 # custom_associations:
 # - "cache_belongs_to"

test/fixtures/minimal/packwerk.yml

@@ -1,4 +1,2 @@
 include:
   - "**/*.rb"
-load_paths:
-  - components/sales/app/models

test/fixtures/skeleton/packwerk.yml

@@ -3,11 +3,5 @@ include:
   - "**/*.{blop,rb}" # To test for duplicated files
 exclude:
   - "**/temp.rb"
-load_paths:
-  - components/platform/app/models
-  - components/sales/app/models
-  - components/timeline/app/models
-  - components/timeline/app/models/concerns
-  - vendor/cache/gems/example/models
 inflections_file: "custom_inflections.yml"
 parallel: false

test/unit/application_validator_test.rb

@@ -26,38 +26,6 @@ class ApplicationValidatorTest < Minitest::Test
       assert result.ok?, result.error_value
     end
 
-    test "check_autoload_path_cache fails on extraneous config load paths" do
-      use_template(:minimal)
-      ApplicationLoadPaths.expects(:extract_relevant_paths).returns([])
-
-      result = validator.check_autoload_path_cache
-
-      refute result.ok?, result.error_value
-      assert_match %r{Extraneous load paths in file:.*components/sales/app/models}m, result.error_value
-    end
-
-    test "check_autoload_path_cache succeeds on duplicated, but correct config load paths" do
-      use_template(:minimal)
-      config.expects(:load_paths).returns(["components/timeline/app/models"] * 2)
-      ApplicationLoadPaths.expects(:extract_relevant_paths).returns(["components/timeline/app/models"])
-
-      result = validator.check_autoload_path_cache
-
-      assert result.ok?, result.error_value
-    end
-
-    test "check_autoload_path_cache fails on missing config load paths" do
-      use_template(:minimal)
-      ApplicationLoadPaths
-        .expects(:extract_relevant_paths)
-        .returns(["components/sales/app/models", "components/timeline/app/models"])
-
-      result = validator.check_autoload_path_cache
-
-      refute result.ok?, result.error_value
-      assert_match %r{Paths missing from file:.*components/timeline/app/models}m, result.error_value
-    end
-
     test "check_package_manifest_syntax returns an error for unknown package keys" do
       use_template(:minimal)
       merge_into_app_yaml_file("package.yml", { "enforce_correctness" => false })

test/unit/cli_test.rb

@@ -24,6 +24,7 @@ class CliTest < Minitest::Test
       offense = Offense.new(file: file_path, message: violation_message)
 
       configuration = Configuration.new({ "parallel" => false })
+      configuration.stubs(load_paths: [])
       RunContext.any_instance.stubs(:process_file).at_least_once.returns([offense])
 
       string_io = StringIO.new
@@ -48,6 +49,8 @@ class CliTest < Minitest::Test
       offense = Offense.new(file: file_path, message: violation_message)
 
       configuration = Configuration.new({ "parallel" => false })
+      configuration.stubs(load_paths: [])
+
       RunContext.any_instance.stubs(:process_file)
         .at_least(2)
         .returns([offense])
@@ -106,7 +109,6 @@ class CliTest < Minitest::Test
       configuration = Configuration.new
       configuration.stubs(
         root_path: @temp_dir,
-        load_paths: ["path"],
         package_paths: "**/",
         custom_associations: ["cached_belongs_to"],
         inflections_file: "config/inflections.yml"
@@ -151,6 +153,7 @@ def show_stale_violations(_offense_collection)
       offense = Offense.new(file: file_path, message: violation_message)
 
       configuration = Configuration.new
+      configuration.stubs(load_paths: [])
       RunContext.any_instance.stubs(:process_file)
         .returns([offense])
 

test/unit/configuration_test.rb

@@ -32,7 +32,6 @@ class ConfigurationTest < Minitest::Test
         "include" => ["xyz/*.rb"],
         "exclude" => ["{exclude_dir,bin,tmp}/**/*"],
         "package_paths" => "**/*/",
-        "load_paths" => ["app/models"],
         "custom_associations" => ["custom_association"],
         "inflections_file" => "custom_inflections.yml",
       }
@@ -43,7 +42,6 @@ class ConfigurationTest < Minitest::Test
       assert_equal ["xyz/*.rb"], configuration.include
       assert_equal ["{exclude_dir,bin,tmp}/**/*"], configuration.exclude
       assert_equal app_dir, configuration.root_path
-      assert_equal ["app/models"], configuration.load_paths
       assert_equal "**/*/", configuration.package_paths
       assert_equal ["custom_association"], configuration.custom_associations
       assert_equal to_app_path("custom_inflections.yml"), configuration.inflections_file
@@ -58,7 +56,6 @@ class ConfigurationTest < Minitest::Test
       assert_equal ["**/*.{rb,rake,erb}"], configuration.include
       assert_equal ["{bin,node_modules,script,tmp,vendor}/**/*"], configuration.exclude
       assert_equal app_dir, configuration.root_path
-      assert_empty configuration.load_paths
       assert_equal "**/", configuration.package_paths
       assert_empty configuration.custom_associations
       assert_equal to_app_path("config/inflections.yml"), configuration.inflections_file

test/unit/constant_discovery_test.rb

@@ -46,7 +46,7 @@ def setup
         discovery.context_for("Sales::RecordNewOrder")
       end
 
-      assert_equal(error.message, "initial error message\n Make sure autoload paths are added to the config file.")
+      assert_equal(error.message, "initial error message")
     end
   end
 end

test/unit/deprecated_references_test.rb

@@ -6,6 +6,7 @@
 module Packwerk
   class DeprecatedReferencesTest < Minitest::Test
     include FactoryHelper
+    include ApplicationFixtureHelper
 
     test "#listed? returns true if constant is violated" do
       violated_reference = build_reference(

test/unit/generators/configuration_file_test.rb

@@ -9,39 +9,22 @@ class ConfigurationFileTest < Minitest::Test
       setup do
         @string_io = StringIO.new
         @temp_dir = Dir.mktmpdir
-        @test_path = "the/path/we/want"
       end
 
       teardown do
         FileUtils.remove_entry(@temp_dir)
       end
 
-      test ".generate creates a configuration file with load paths if available" do
-        load_paths = [@test_path]
-        generated_file_path = File.join(@temp_dir, Packwerk::Configuration::DEFAULT_CONFIG_PATH)
-
-        assert(Packwerk::Generators::ConfigurationFile.generate(
-          load_paths: load_paths,
-          root: @temp_dir,
-          out: @string_io
-        ))
-        assert(File.exist?(generated_file_path))
-
-        contents = YAML.load_file(generated_file_path)
-        assert_equal(load_paths, contents["load_paths"])
-      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)
-        assert(Packwerk::Generators::ConfigurationFile.generate(load_paths: [], root: @temp_dir, out: @string_io))
+        assert(Packwerk::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(
-            load_paths: [@test_path],
             root: @temp_dir,
             out: @string_io
           ))