Skip to content

Commit

Permalink
Remove instance_eval from InspectionTree (#210)
Browse files Browse the repository at this point in the history
The InspectionTree class defines methods that create nodes in the tree.
Some of these methods take a block which is `instance_eval`'ed inside of
a new subtree. This use of `instance_eval` may seem to create a very
clean and easy to use API on the surface, but it quickly breaks down if,
as you are defining your InspectionTree, you need to extract methods
inside of your InspectionTreeBuilder and then reference them. _Not_
`instance_eval`'ing creates footguns, but it also removes the magic and
mystery around how InspectionTree works.
  • Loading branch information
mcmire authored Feb 1, 2024
1 parent 599ffbd commit 181b102
Show file tree
Hide file tree
Showing 22 changed files with 432 additions and 253 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,27 +8,39 @@ def self.applies_to?(value)
end

def call
SuperDiff::ObjectInspection::InspectionTree.new do
as_lines_when_rendering_to_lines(collection_bookend: :open) do
add_text { |object| "#<#{object.class} " }
SuperDiff::ObjectInspection::InspectionTree.new do |t1|
t1.as_lines_when_rendering_to_lines(
collection_bookend: :open
) do |t2|
t2.add_text "#<#{object.class} "

when_rendering_to_lines { add_text "{" }
# stree-ignore
t2.when_rendering_to_lines do |t3|
t3.add_text "{"
end
end

nested do |object|
insert_separated_list(
t1.nested do |t2|
t2.insert_separated_list(
["id"] + (object.attributes.keys.sort - ["id"])
) do |name|
as_prefix_when_rendering_to_lines { add_text "#{name}: " }
) do |t3, name|
t3.as_prefix_when_rendering_to_lines do |t4|
t4.add_text "#{name}: "
end

add_inspection_of object.read_attribute(name)
t3.add_inspection_of object.read_attribute(name)
end
end

as_lines_when_rendering_to_lines(collection_bookend: :close) do
when_rendering_to_lines { add_text "}" }
t1.as_lines_when_rendering_to_lines(
collection_bookend: :close
) do |t2|
# stree-ignore
t2.when_rendering_to_lines do |t3|
t3.add_text "}"
end

add_text ">"
t2.add_text ">"
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,24 @@ def self.applies_to?(value)
end

def call
SuperDiff::ObjectInspection::InspectionTree.new do
as_lines_when_rendering_to_lines(collection_bookend: :open) do
add_text "#<ActiveRecord::Relation ["
SuperDiff::ObjectInspection::InspectionTree.new do |t1|
# stree-ignore
t1.as_lines_when_rendering_to_lines(
collection_bookend: :open
) do |t2|
t2.add_text "#<ActiveRecord::Relation ["
end

nested { |array| insert_array_inspection_of(array) }
# stree-ignore
t1.nested do |t2|
t2.insert_array_inspection_of(object)
end

as_lines_when_rendering_to_lines(collection_bookend: :close) do
add_text "]>"
# stree-ignore
t1.as_lines_when_rendering_to_lines(
collection_bookend: :close
) do |t2|
t2.add_text "]>"
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,34 @@ def self.applies_to?(value)
end

def call
SuperDiff::ObjectInspection::InspectionTree.new do
as_lines_when_rendering_to_lines(collection_bookend: :open) do
add_text "#<HashWithIndifferentAccess {"
SuperDiff::ObjectInspection::InspectionTree.new do |t1|
# stree-ignore
t1.as_lines_when_rendering_to_lines(
collection_bookend: :open
) do |t2|
t2.add_text "#<HashWithIndifferentAccess {"
end

when_rendering_to_string { add_text " " }
# stree-ignore
t1.when_rendering_to_string do |t2|
t2.add_text " "
end

nested { |hash| insert_hash_inspection_of(hash) }
# stree-ignore
t1.nested do |t2|
t2.insert_hash_inspection_of(object)
end

when_rendering_to_string { add_text " " }
# stree-ignore
t1.when_rendering_to_string do |t2|
t2.add_text " "
end

as_lines_when_rendering_to_lines(collection_bookend: :close) do
add_text "}>"
# stree-ignore
t1.as_lines_when_rendering_to_lines(
collection_bookend: :close
) do |t2|
t2.add_text "}>"
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,34 @@ def self.applies_to?(value)
end

def call
SuperDiff::ObjectInspection::InspectionTree.new do
as_lines_when_rendering_to_lines(collection_bookend: :open) do
add_text "#<OrderedOptions {"
SuperDiff::ObjectInspection::InspectionTree.new do |t1|
# stree-ignore
t1.as_lines_when_rendering_to_lines(
collection_bookend: :open
) do |t2|
t2.add_text "#<OrderedOptions {"
end

when_rendering_to_string { add_text " " }
# stree-ignore
t1.when_rendering_to_string do |t2|
t2.add_text " "
end

nested do |ordered_options|
insert_hash_inspection_of(ordered_options.to_hash)
# stree-ignore
t1.nested do |t2|
t2.insert_hash_inspection_of(object.to_hash)
end

when_rendering_to_string { add_text " " }
# stree-ignore
t1.when_rendering_to_string do |t2|
t2.add_text " "
end

as_lines_when_rendering_to_lines(collection_bookend: :close) do
add_text "}>"
# stree-ignore
t1.as_lines_when_rendering_to_lines(
collection_bookend: :close
) do |t2|
t2.add_text "}>"
end
end
end
Expand Down
45 changes: 25 additions & 20 deletions lib/super_diff/object_inspection/inspection_tree.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ def initialize(disallowed_node_names: [], &block)
@disallowed_node_names = disallowed_node_names
@nodes = []

instance_eval(&block) if block
evaluate_block(&block) if block
end

Nodes.registry.each do |node_class|
Expand All @@ -26,8 +26,7 @@ def before_each_callbacks

def render_to_string(object)
nodes.reduce("") do |string, node|
result = node.render_to_string(object)
string + result
string + node.render_to_string(object)
end
end

Expand All @@ -52,43 +51,45 @@ def render_to_lines(object, type:, indentation_level:)
.first
end

def evaluate_block(object, &block)
instance_exec(object, &block)
def evaluate_block(*args, &block)
block.call(self, *args)
end

def insert_array_inspection_of(array)
insert_separated_list(array) do |value|
insert_separated_list(array) do |t, value|
# Have to do these shenanigans so that if value is a hash, Ruby
# doesn't try to interpret it as keyword args
if SuperDiff::Helpers.ruby_version_matches?(">= 2.7.1")
add_inspection_of(value, **{})
t.add_inspection_of(value, **{})
else
add_inspection_of(*[value, {}])
t.add_inspection_of(*[value, {}])
end
end
end

def insert_hash_inspection_of(hash)
keys = hash.keys

format_keys_as_kwargs = keys.all? { |key| key.is_a?(Symbol) }

insert_separated_list(keys) do |key|
insert_separated_list(keys) do |t1, key|
if format_keys_as_kwargs
as_prefix_when_rendering_to_lines { add_text "#{key}: " }
# stree-ignore
t1.as_prefix_when_rendering_to_lines do |t2|
t2.add_text "#{key}: "
end
else
as_prefix_when_rendering_to_lines do
add_inspection_of key, as_lines: false
add_text " => "
t1.as_prefix_when_rendering_to_lines do |t2|
t2.add_inspection_of key, as_lines: false
t2.add_text " => "
end
end

# Have to do these shenanigans so that if hash[key] is a hash, Ruby
# doesn't try to interpret it as keyword args
if SuperDiff::Helpers.ruby_version_matches?(">= 2.7.1")
add_inspection_of(hash[key], **{})
t1.add_inspection_of(hash[key], **{})
else
add_inspection_of(*[hash[key], {}])
t1.add_inspection_of(*[hash[key], {}])
end
end
end
Expand All @@ -97,10 +98,14 @@ def insert_separated_list(enumerable, &block)
enumerable.each_with_index do |value, index|
as_lines_when_rendering_to_lines(
add_comma: index < enumerable.size - 1
) do
when_rendering_to_string { add_text " " } if index > 0

evaluate_block(value, &block)
) do |t1|
if index > 0
# stree-ignore
t1.when_rendering_to_string do |t2|
t2.add_text " "
end
end
t1.evaluate_block(value, &block)
end
end
end
Expand Down
43 changes: 31 additions & 12 deletions lib/super_diff/object_inspection/inspection_tree_builders/array.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,46 @@ def self.applies_to?(value)
end

def call
empty = -> { object.empty? }
nonempty = -> { !object.empty? }

InspectionTree.new do
only_when empty do
as_lines_when_rendering_to_lines { add_text "[]" }
InspectionTree.new do |t1|
t1.only_when empty do |t2|
# stree-ignore
t2.as_lines_when_rendering_to_lines do |t3|
t3.add_text "[]"
end
end

only_when nonempty do
as_lines_when_rendering_to_lines(collection_bookend: :open) do
add_text "["
t1.only_when nonempty do |t2|
# stree-ignore
t2.as_lines_when_rendering_to_lines(
collection_bookend: :open
) do |t3|
t3.add_text "["
end

nested { |array| insert_array_inspection_of(array) }
# stree-ignore
t2.nested do |t3|
t3.insert_array_inspection_of(object)
end

as_lines_when_rendering_to_lines(collection_bookend: :close) do
add_text "]"
# stree-ignore
t2.as_lines_when_rendering_to_lines(
collection_bookend: :close
) do |t3|
t3.add_text "]"
end
end
end
end

private

def empty
-> { object.empty? }
end

def nonempty
-> { !object.empty? }
end
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,31 @@ def self.applies_to?(value)
end

def call
InspectionTree.new do
as_lines_when_rendering_to_lines(collection_bookend: :open) do
add_text { |object| "#<#{object.class} " }
InspectionTree.new do |t1|
t1.as_lines_when_rendering_to_lines(
collection_bookend: :open
) do |t2|
t2.add_text "#<#{object.class} "

when_rendering_to_lines { add_text "{" }
# stree-ignore
t2.when_rendering_to_lines do |t3|
t3.add_text "{"
end
end

nested do |object|
insert_hash_inspection_of(object.attributes_for_super_diff)
t1.nested do |t2|
t2.insert_hash_inspection_of(object.attributes_for_super_diff)
end

as_lines_when_rendering_to_lines(collection_bookend: :close) do
when_rendering_to_lines { add_text "}" }
t1.as_lines_when_rendering_to_lines(
collection_bookend: :close
) do |t2|
# stree-ignore
t2.when_rendering_to_lines do |t3|
t3.add_text "}"
end

add_text ">"
t2.add_text ">"
end
end
end
Expand Down
Loading

0 comments on commit 181b102

Please sign in to comment.