Skip to content

Commit

Permalink
Support Active Model Serializers in association inspector
Browse files Browse the repository at this point in the history
Other gems like ActiveModel::Serializers define association methods that
reference serializers.
  • Loading branch information
gmcgibbon committed Nov 16, 2023
1 parent b97f572 commit 6723041
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 18 deletions.
50 changes: 43 additions & 7 deletions lib/packwerk/association_inspector.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,26 @@ def initialize(inflector:, custom_associations: Set.new)
end
def constant_name_from_node(node, ancestors:)
return unless NodeHelpers.method_call?(node)
return unless association?(node)
return unless association_method?(node)

case type(node)
when :active_record
active_record_constant_name(node)
when :active_model_serializers
active_model_serializers_constant_name(node)
else
raise TypeError
end
end

private

sig { params(node: AST::Node).returns(T.nilable(String)) }
def active_record_constant_name(node)
arguments = NodeHelpers.method_arguments(node)
return unless (association_name = association_name(arguments))

if (class_name_node = custom_class_name(arguments))
if (class_name_node = has_value(arguments, key: :class_name))
return unless NodeHelpers.string?(class_name_node)

NodeHelpers.literal_value(class_name_node)
Expand All @@ -46,20 +60,42 @@ def constant_name_from_node(node, ancestors:)
end
end

private
sig { params(node: AST::Node).returns(T.nilable(String)) }
def active_model_serializers_constant_name(node)
arguments = NodeHelpers.method_arguments(node)
return unless (association_name = association_name(arguments))

if (class_name_node = has_value(arguments, key: :serializer))
return unless NodeHelpers.constant?(class_name_node)

T.cast(NodeHelpers.literal_value(class_name_node), String)
else
"#{@inflector.classify(association_name.to_s)}Serializer"
end
end

sig { params(node: AST::Node).returns(T.nilable(Symbol)) }
def type(node)
case NodeHelpers.source_location(node).match(%r{\bapp/(.*)/}).to_a.last
when "models"
:active_record
when "serializers"
:active_model_serializers
end
end

sig { params(node: AST::Node).returns(T::Boolean) }
def association?(node)
def association_method?(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)
sig { params(arguments: T::Array[AST::Node], key: Symbol).returns(T.nilable(AST::Node)) }
def has_value(arguments, key:)
association_options = arguments.detect { |n| NodeHelpers.hash?(n) }
return unless association_options

NodeHelpers.value_from_hash(association_options, :class_name)
NodeHelpers.value_from_hash(association_options, key)
end

sig { params(arguments: T::Array[AST::Node]).returns(T.any(T.nilable(Symbol), T.nilable(String))) }
Expand Down
15 changes: 11 additions & 4 deletions lib/packwerk/node_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ class TypeError < ArgumentError; end
class << self
extend T::Sig

sig { params(node: AST::Node).returns(String) }
def source_location(node)
T.cast(node, Parser::AST::Node).loc.selector.to_s
end

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)
Expand Down Expand Up @@ -88,15 +93,17 @@ def enclosing_namespace_path(starting_node, ancestors:)
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)
sig { params(node: AST::Node).returns(T.any(String, Symbol)) }
def literal_value(node)
case type_of(node)
when STRING, SYMBOL
# (str "foo")
# "'foo'"
# (sym :foo)
# ":foo"
string_or_symbol_node.children[0]
node.children[0]
when CONSTANT
constant_name(node)
else
raise TypeError
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class CustomExecutableIntegrationTest < Minitest::Test
end

test "'packwerk check' with violations only in nested packages has different outcomes per variant" do
open_app_file(TIMELINE_PATH.join("nested", "timeline_comment.rb")) do |file|
open_app_file(TIMELINE_PATH.join("nested", "app", "models", "timeline_comment.rb")) do |file|
file.write("class TimelineComment; belongs_to :order, class_name: '::Order'; end")
file.flush
end
Expand All @@ -54,7 +54,7 @@ class CustomExecutableIntegrationTest < Minitest::Test
end

test "'packwerk check' with failures in different parts of the app has different outcomes per variant" do
open_app_file(TIMELINE_PATH.join("nested", "timeline_comment.rb")) do |file|
open_app_file(TIMELINE_PATH.join("nested", "app", "models", "timeline_comment.rb")) do |file|
file.write("class TimelineComment; belongs_to :order, class_name: '::Order'; end")
file.flush
end
Expand Down
1 change: 1 addition & 0 deletions test/support/packwerk/application_fixture_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ def remove_app_entry(relative_path)

def open_app_file(path, mode: "w+", &block)
expanded_path = to_app_path(File.join(path))
FileUtils.mkdir_p(Pathname.new(expanded_path).dirname)
File.open(expanded_path, mode, &block)
end

Expand Down
4 changes: 2 additions & 2 deletions test/support/packwerk/parser_test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
module Packwerk
module ParserTestHelper
class << self
def parse(source)
Parsers::Ruby.new.call(io: StringIO.new(source))
def parse(source, file_path: "<unknown>")
Parsers::Ruby.new.call(io: StringIO.new(source), file_path: file_path)
end
end
end
Expand Down
41 changes: 38 additions & 3 deletions test/unit/packwerk/association_inspector_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,43 @@ class AssociationInspectorTest < Minitest::Test
assert_equal "Order", inspector.constant_name_from_node(node, ancestors: [])
end

test "finds target constant for association that is possessive" do
node = parse("belongs_to :order")
inspector = AssociationInspector.new(inflector: @inflector)

assert_equal "Order", inspector.constant_name_from_node(node, ancestors: [])
end

test "finds target constant for association that is HABTM" do
node = parse("has_and_belongs_to_many :orders")
inspector = 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)

assert_equal "Order", inspector.constant_name_from_node(node, ancestors: [])
end

test "finds target constant for simple Active Model Serializers association" do
node = parse("has_one :order", file_path: "app/serializers/my_model.rb")
inspector = AssociationInspector.new(inflector: @inflector)

assert_equal "OrderSerializer", inspector.constant_name_from_node(node, ancestors: [])
end

test "finds target constant for simple Active Model Serializers association" do
node = parse("has_one :order, serializer: My::CustomSerializer", file_path: "app/serializers/my_model.rb")
inspector = AssociationInspector.new(inflector: @inflector)

assert_equal "My::CustomSerializer", 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' }")
node = parse("has_one :cool_order, class_name: 'Order'")
inspector = AssociationInspector.new(inflector: @inflector)

assert_equal "Order", inspector.constant_name_from_node(node, ancestors: [])
Expand Down Expand Up @@ -61,8 +96,8 @@ class AssociationInspectorTest < Minitest::Test

private

def parse(statement)
ParserTestHelper.parse(statement)
def parse(statement, file_path: "app/models/my_model.rb")
ParserTestHelper.parse(statement, file_path: file_path)
end
end
end

0 comments on commit 6723041

Please sign in to comment.