Merge pull request #380 from Shopify/support_non_ar_association

Add assocaitions_exclude to exclude files from association inspection

USAGE.md

@@ -76,6 +76,7 @@ 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 |
+| associations_exclude | N/A                                       | list of patterns for folder paths to exclude from association inspection |
 | package_paths        | **/                                       | a single pattern or a list of patterns to find package configuration files, see: [Defining packages](#Defining-packages) |
 | custom_associations  | N/A                                       | list of custom associations, if any |
 | parallel             | true                                      | when true, fork code parsing out to subprocesses |

lib/packwerk/association_inspector.rb

@@ -19,19 +19,27 @@ class AssociationInspector
       CustomAssociations
     )
 
-    sig { params(inflector: T.class_of(ActiveSupport::Inflector), custom_associations: CustomAssociations).void }
-    def initialize(inflector:, custom_associations: Set.new)
+    sig do
+      params(
+        inflector: T.class_of(ActiveSupport::Inflector),
+        custom_associations: CustomAssociations,
+        excluded_files: T::Set[String]
+      ).void
+    end
+    def initialize(inflector:, custom_associations: Set.new, excluded_files: Set.new)
       @inflector = inflector
       @associations = T.let(RAILS_ASSOCIATIONS + custom_associations, CustomAssociations)
+      @excluded_files = T.let(excluded_files, T::Set[String])
     end
 
     sig do
       override
-        .params(node: AST::Node, ancestors: T::Array[AST::Node])
+        .params(node: AST::Node, ancestors: T::Array[AST::Node], relative_file: String)
         .returns(T.nilable(String))
     end
-    def constant_name_from_node(node, ancestors:)
+    def constant_name_from_node(node, ancestors:, relative_file:)
       return unless NodeHelpers.method_call?(node)
+      return if excluded?(relative_file)
       return unless association?(node)
 
       arguments = NodeHelpers.method_arguments(node)
@@ -48,6 +56,11 @@ def constant_name_from_node(node, ancestors:)
 
     private
 
+    sig { params(relative_file: String).returns(T::Boolean) }
+    def excluded?(relative_file)
+      @excluded_files.include?(relative_file)
+    end
+
     sig { params(node: AST::Node).returns(T::Boolean) }
     def association?(node)
       method_name = NodeHelpers.method_name(node)

lib/packwerk/configuration.rb

@@ -54,6 +54,9 @@ def from_packwerk_config(path)
     sig { returns(T::Array[Symbol]) }
     attr_reader(:custom_associations)
 
+    sig { returns(T::Array[String]) }
+    attr_reader(:associations_exclude)
+
     sig { returns(T.nilable(String)) }
     attr_reader(:config_path)
 
@@ -76,6 +79,7 @@ def initialize(configs = {}, config_path: nil)
       @root_path = T.let(File.expand_path(root), String)
       @package_paths = T.let(configs["package_paths"] || "**/", T.any(String, T::Array[String]))
       @custom_associations = T.let((configs["custom_associations"] || []).map(&:to_sym), T::Array[Symbol])
+      @associations_exclude = T.let(configs["associations_exclude"] || [], T::Array[String])
       @parallel = T.let(configs.key?("parallel") ? configs["parallel"] : true, T::Boolean)
       @cache_enabled = T.let(configs.key?("cache") ? configs["cache"] : false, T::Boolean)
       @cache_directory = T.let(Pathname.new(configs["cache_directory"] || "tmp/cache/packwerk"), Pathname)

lib/packwerk/const_node_inspector.rb

@@ -9,10 +9,10 @@ class ConstNodeInspector
 
     sig do
       override
-        .params(node: AST::Node, ancestors: T::Array[AST::Node])
+        .params(node: AST::Node, ancestors: T::Array[AST::Node], relative_file: String)
         .returns(T.nilable(String))
     end
-    def constant_name_from_node(node, ancestors:)
+    def constant_name_from_node(node, ancestors:, relative_file:)
       return nil unless NodeHelpers.constant?(node)
 
       parent = ancestors.first

lib/packwerk/constant_name_inspector.rb

@@ -13,10 +13,10 @@ module ConstantNameInspector
 
     sig do
       abstract
-        .params(node: ::AST::Node, ancestors: T::Array[::AST::Node])
+        .params(node: ::AST::Node, ancestors: T::Array[::AST::Node], relative_file: String)
         .returns(T.nilable(String))
     end
-    def constant_name_from_node(node, ancestors:); end
+    def constant_name_from_node(node, ancestors:, relative_file:); end
   end
 
   private_constant :ConstantNameInspector

lib/packwerk/reference_extractor.rb

@@ -80,7 +80,12 @@ 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 = inspect_node(
+          inspector,
+          node: node,
+          ancestors: ancestors,
+          relative_file: relative_file
+        )
 
         break if constant_name
       end
@@ -97,6 +102,29 @@ def reference_from_node(node, ancestors:, relative_file:)
 
     private
 
+    sig do
+      params(
+        inspector: ConstantNameInspector,
+        node: Parser::AST::Node,
+        ancestors: T::Array[Parser::AST::Node],
+        relative_file: String
+      ).returns(T.nilable(String))
+    end
+    def inspect_node(inspector, node:, ancestors:, relative_file:)
+      inspector.constant_name_from_node(node, ancestors: ancestors, relative_file: relative_file)
+    rescue ArgumentError => error
+      if error.message == "unknown keyword: :relative_file"
+        T.unsafe(inspector).constant_name_from_node(node, ancestors: ancestors).tap do
+          warn(<<~MSG.squish)
+            #{T.cast(inspector, Object).class}#reference_from_node without a relative_file: keyword
+            argument is deprecated and will be required in Packwerk 3.1.1.
+          MSG
+        end
+      else
+        raise
+      end
+    end
+
     sig do
       params(
         constant_name: String,

lib/packwerk/run_context.rb

@@ -15,14 +15,13 @@ class << self
         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,
+          inflector: ActiveSupport::Inflector,
           custom_associations: configuration.custom_associations,
+          associations_exclude: configuration.associations_exclude,
           cache_enabled: configuration.cache_enabled?,
           cache_directory: configuration.cache_directory,
           config_path: configuration.config_path,
@@ -39,6 +38,7 @@ def from_configuration(configuration)
         config_path: T.nilable(String),
         package_paths: T.nilable(T.any(T::Array[String], String)),
         custom_associations: AssociationInspector::CustomAssociations,
+        associations_exclude: T::Array[String],
         checkers: T::Array[Checker],
         cache_enabled: T::Boolean,
       ).void
@@ -51,6 +51,7 @@ def initialize(
       config_path: nil,
       package_paths: nil,
       custom_associations: [],
+      associations_exclude: [],
       checkers: Checker.all,
       cache_enabled: false
     )
@@ -59,6 +60,7 @@ def initialize(
       @package_paths = package_paths
       @inflector = inflector
       @custom_associations = custom_associations
+      @associations_exclude = associations_exclude
       @checkers = checkers
       @cache_enabled = cache_enabled
       @cache_directory = cache_directory
@@ -128,9 +130,18 @@ def resolver
     def constant_name_inspectors
       [
         ConstNodeInspector.new,
-        AssociationInspector.new(inflector: @inflector, custom_associations: @custom_associations),
+        AssociationInspector.new(
+          inflector: @inflector,
+          custom_associations: @custom_associations,
+          excluded_files: relative_files_for_globs(@associations_exclude),
+        ),
       ]
     end
+
+    sig { params(relative_globs: T::Array[String]).returns(FilesForProcessing::RelativeFileSet) }
+    def relative_files_for_globs(relative_globs)
+      Set.new(relative_globs.flat_map { |glob| Dir[glob] })
+    end
   end
 
   private_constant :RunContext

test/unit/packwerk/association_inspector_test.rb

@@ -14,49 +14,62 @@ class AssociationInspectorTest < Minitest::Test
       node = parse("has_lots :order")
       inspector = AssociationInspector.new(inflector: @inflector, custom_associations: [:has_lots])
 
-      assert_equal "Order", inspector.constant_name_from_node(node, ancestors: [])
+      assert_equal "Order", inspector.constant_name_from_node(node, ancestors: [], relative_file: "")
     end
 
     test "finds target constant for simple association" do
       node = parse("has_one :order")
       inspector = AssociationInspector.new(inflector: @inflector)
 
-      assert_equal "Order", inspector.constant_name_from_node(node, ancestors: [])
+      assert_equal "Order", inspector.constant_name_from_node(node, ancestors: [], relative_file: "")
     end
 
     test "finds target constant for association that pluralizes" do
       node = parse("has_many :orders")
       inspector = AssociationInspector.new(inflector: @inflector)
 
-      assert_equal "Order", inspector.constant_name_from_node(node, ancestors: [])
+      assert_equal "Order", inspector.constant_name_from_node(node, ancestors: [], relative_file: "")
     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)
 
-      assert_equal "Order", inspector.constant_name_from_node(node, ancestors: [])
+      assert_equal "Order", inspector.constant_name_from_node(node, ancestors: [], relative_file: "")
     end
 
     test "rejects method calls that are not associations" do
       node = parse('puts "Hello World"')
       inspector = AssociationInspector.new(inflector: @inflector)
 
-      assert_nil inspector.constant_name_from_node(node, ancestors: [])
+      assert_nil inspector.constant_name_from_node(node, ancestors: [], relative_file: "")
     end
 
     test "gives up on metaprogrammed associations" do
       node = parse("has_one association_name")
       inspector = AssociationInspector.new(inflector: @inflector)
 
-      assert_nil inspector.constant_name_from_node(node, ancestors: [])
+      assert_nil inspector.constant_name_from_node(node, ancestors: [], relative_file: "")
     end
 
     test "gives up on dynamic class name" do
       node = parse("has_one :order, class_name: Order.name")
       inspector = AssociationInspector.new(inflector: @inflector)
 
-      assert_nil inspector.constant_name_from_node(node, ancestors: [])
+      assert_nil inspector.constant_name_from_node(node, ancestors: [], relative_file: "")
+    end
+
+    test "gives up on excluded file node" do
+      node = parse("has_one :order")
+      inspector = AssociationInspector.new(
+        inflector: @inflector, excluded_files: Set["app/serializers/my_serializer.rb"]
+      )
+
+      assert_nil inspector.constant_name_from_node(
+        node,
+        ancestors: [],
+        relative_file: "app/serializers/my_serializer.rb",
+      )
     end
 
     private

test/unit/packwerk/const_node_inspector_test.rb

@@ -13,31 +13,31 @@ class ConstNodeInspectorTest < ActiveSupport::TestCase
     test "#constant_name_from_node should ignore any non-const nodes" do
       node = parse("a = 1 + 1")
 
-      constant_name = @inspector.constant_name_from_node(node, ancestors: [])
+      constant_name = @inspector.constant_name_from_node(node, ancestors: [], relative_file: "")
 
       assert_nil constant_name
     end
 
     test "#constant_name_from_node should return correct name for const node" do
       node = parse("Order")
 
-      constant_name = @inspector.constant_name_from_node(node, ancestors: [])
+      constant_name = @inspector.constant_name_from_node(node, ancestors: [], relative_file: "")
 
       assert_equal "Order", constant_name
     end
 
     test "#constant_name_from_node should return correct name for fully-qualified const node" do
       node = parse("::Order")
 
-      constant_name = @inspector.constant_name_from_node(node, ancestors: [])
+      constant_name = @inspector.constant_name_from_node(node, ancestors: [], relative_file: "")
 
       assert_equal "::Order", constant_name
     end
 
     test "#constant_name_from_node should return correct name for compact const node" do
       node = parse("Sales::Order")
 
-      constant_name = @inspector.constant_name_from_node(node, ancestors: [])
+      constant_name = @inspector.constant_name_from_node(node, ancestors: [], relative_file: "")
 
       assert_equal "Sales::Order", constant_name
     end
@@ -46,7 +46,7 @@ class ConstNodeInspectorTest < ActiveSupport::TestCase
       parent = parse("class Order; end")
       node = NodeHelpers.each_child(parent).entries[0]
 
-      constant_name = @inspector.constant_name_from_node(node, ancestors: [parent])
+      constant_name = @inspector.constant_name_from_node(node, ancestors: [parent], relative_file: "")
 
       assert_equal "::Order", constant_name
     end
@@ -58,7 +58,7 @@ class ConstNodeInspectorTest < ActiveSupport::TestCase
       ) # module node; second child is the body of the module
       node = NodeHelpers.each_child(parent).entries[0] # class node; first child is constant
 
-      constant_name = @inspector.constant_name_from_node(node, ancestors: [parent, grandparent])
+      constant_name = @inspector.constant_name_from_node(node, ancestors: [parent, grandparent], relative_file: "")
 
       assert_equal "::Foo::Bar::Sales::Order", constant_name
     end
@@ -68,7 +68,7 @@ class ConstNodeInspectorTest < ActiveSupport::TestCase
       parent = T.must(NodeHelpers.each_child(grandparent).entries[1]) # setup do self.class::HEADERS end
       node = NodeHelpers.each_child(parent).entries[2] # self.class::HEADERS
 
-      constant_name = @inspector.constant_name_from_node(node, ancestors: [parent, grandparent])
+      constant_name = @inspector.constant_name_from_node(node, ancestors: [parent, grandparent], relative_file: "")
 
       assert_nil constant_name
     end

test/unit/packwerk/reference_extractor_test.rb

@@ -211,6 +211,21 @@ def teardown
       assert_equal "::Order", reference.constant.name
     end
 
+    test "constant name inspector without file name kwarg is deprecated but works" do
+      _, error_output = capture_io do
+        process(
+          ":orders",
+          "components/timeline/app/models/entry.rb",
+          [DeprecatedInspector.new],
+        )
+      end
+
+      assert_equal(<<~MSG.squish, error_output.chomp)
+        Packwerk::ReferenceExtractorTest::DeprecatedInspector#reference_from_node without
+        a relative_file: keyword argument is deprecated and will be required in Packwerk 3.1.1.
+      MSG
+    end
+
     private
 
     class DummyAssociationInspector
@@ -222,7 +237,7 @@ def initialize(association: false, reference_name: "Dummy", expected_args: nil)
         @expected_args = expected_args
       end
 
-      def constant_name_from_node(node, ancestors:)
+      def constant_name_from_node(node, ancestors:, relative_file:)
         return nil unless @association
         return nil unless NodeHelpers.method_call?(node)
 
@@ -235,6 +250,14 @@ def constant_name_from_node(node, ancestors:)
       end
     end
 
+    class DeprecatedInspector
+      T.unsafe(self).include(ConstantNameInspector)
+
+      def constant_name_from_node(node, ancestors:)
+        "Something"
+      end
+    end
+
     DEFAULT_INSPECTORS = [ConstNodeInspector.new, DummyAssociationInspector.new]
 
     def process(code, file_path, constant_name_inspectors = DEFAULT_INSPECTORS)