Skip to content

Commit 85e4817

Browse files
committed
Handle recursive data structures
When inspecting or diffing a data structure that refers to itself, replace that data structure with "∙∙∙" to prevent infinite recursion.
1 parent 147825e commit 85e4817

File tree

10 files changed

+335
-39
lines changed

10 files changed

+335
-39
lines changed

README.md

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,24 @@ JRuby >= 9.2.x.
174174

175175
[travis]: http://travis-ci.org/mcmire/super_diff
176176

177+
## Inspiration/Thanks
178+
179+
In developing this gem I made use of or was heavily inspired by these libraries:
180+
181+
* [Diff::LCS][diff-lcs], the library I started with in the [original version of
182+
this gem][original-version] (made in 2011!)
183+
* The pretty-printing algorithms and API within [PrettyPrinter][pretty-printer]
184+
and [AwesomePrint][awesome-print], from which I borrowed ideas to develop
185+
the [inspectors][inspection-tree].
186+
187+
Thank you so much!
188+
189+
[original-version]: https://github.com/mcmire/super_diff/tree/old-master
190+
[diff-lcs]: https://github.com/halostatue/diff-lcs
191+
[pretty-printer]: https://github.com/ruby/ruby/tree/master/lib
192+
[awesome-print]: https://github.com/awesome-print/awesome_print
193+
[inspection-tree]: https://github.com/mcmire/super_diff/blob/master/lib/super_diff/object_inspection/inspection_tree.rb
194+
177195
## Copyright/License
178196

179197
© 2018-2019 Elliot Winkler, released under the [MIT license](LICENSE).

lib/super_diff.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,5 @@ module SuperDiff
2222
autoload :OperationalSequencers, "super_diff/operational_sequencers"
2323
autoload :OperationSequences, "super_diff/operation_sequences"
2424
autoload :Operations, "super_diff/operations"
25+
autoload :RecursionGuard, "super_diff/recursion_guard"
2526
end

lib/super_diff/diff_formatters/collection.rb

Lines changed: 51 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -37,36 +37,72 @@ def lines
3737
def contents
3838
operations.map do |operation|
3939
if operation.name == :change
40+
handle_change_operation(operation)
41+
else
42+
handle_non_change_operation(operation)
43+
end
44+
end
45+
end
46+
47+
def handle_change_operation(operation)
48+
SuperDiff::RecursionGuard.guarding_recursion_of(
49+
operation.left_collection,
50+
operation.right_collection,
51+
) do |already_seen|
52+
if already_seen
53+
raise "Infinite recursion!"
54+
else
4055
operation.child_operations.to_diff(
4156
indent_level: indent_level + 1,
4257
collection_prefix: build_item_prefix.call(operation),
4358
add_comma: operation.should_add_comma_after_displaying?,
4459
)
45-
else
46-
icon = ICONS.fetch(operation.name, " ")
47-
style_name = STYLES.fetch(operation.name, :normal)
48-
chunk = build_chunk(
49-
operation.value,
50-
prefix: build_item_prefix.call(operation),
51-
icon: icon,
52-
)
60+
end
61+
end
62+
end
5363

54-
if operation.should_add_comma_after_displaying?
55-
chunk << ","
56-
end
64+
def handle_non_change_operation(operation)
65+
icon = ICONS.fetch(operation.name, " ")
66+
style_name = STYLES.fetch(operation.name, :normal)
67+
chunk = build_chunk_for(
68+
operation,
69+
prefix: build_item_prefix.call(operation),
70+
icon: icon,
71+
)
5772

58-
style_chunk(style_name, chunk)
59-
end
73+
if operation.should_add_comma_after_displaying?
74+
chunk << ","
6075
end
76+
77+
style_chunk(style_name, chunk)
6178
end
6279

63-
def build_chunk(value, prefix:, icon:)
80+
def build_chunk_for(operation, prefix:, icon:)
81+
if operation.value.equal?(operation.collection)
82+
build_chunk_from_string(
83+
SuperDiff::RecursionGuard::PLACEHOLDER,
84+
prefix: build_item_prefix.call(operation),
85+
icon: icon,
86+
)
87+
else
88+
build_chunk_by_inspecting(
89+
operation.value,
90+
prefix: build_item_prefix.call(operation),
91+
icon: icon,
92+
)
93+
end
94+
end
95+
96+
def build_chunk_by_inspecting(value, prefix:, icon:)
6497
inspection = ObjectInspection.inspect(
6598
value,
6699
as_single_line: false,
67100
)
101+
build_chunk_from_string(inspection, prefix: prefix, icon: icon)
102+
end
68103

69-
inspection.split("\n").
104+
def build_chunk_from_string(value, prefix:, icon:)
105+
value.split("\n").
70106
map.with_index { |line, index|
71107
[
72108
icon,

lib/super_diff/object_inspection.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
module SuperDiff
22
module ObjectInspection
33
autoload :InspectionTree, "super_diff/object_inspection/inspection_tree"
4+
autoload :Inspector, "super_diff/object_inspection/inspector"
45
autoload :Inspectors, "super_diff/object_inspection/inspectors"
56
autoload :Map, "super_diff/object_inspection/map"
67
autoload :Nodes, "super_diff/object_inspection/nodes"
@@ -10,7 +11,8 @@ class << self
1011
end
1112

1213
def self.inspect(object, as_single_line:, indent_level: 0)
13-
map.call(object).evaluate(
14+
Inspector.call(
15+
map,
1416
object,
1517
as_single_line: as_single_line,
1618
indent_level: indent_level,
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
module SuperDiff
2+
module ObjectInspection
3+
class Inspector
4+
extend AttrExtras.mixin
5+
6+
method_object :map, :object, [:indent_level!, :as_single_line!]
7+
8+
def call
9+
SuperDiff::RecursionGuard.substituting_recursion_of(object) do
10+
inspector.evaluate(
11+
object,
12+
as_single_line: as_single_line,
13+
indent_level: indent_level,
14+
)
15+
end
16+
end
17+
18+
private
19+
20+
attr_query :as_single_line?
21+
22+
def inspector
23+
@_inspector ||= map.call(object)
24+
end
25+
end
26+
end
27+
end

lib/super_diff/operational_sequencers/array.rb

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ def lcs_callbacks
1919
actual: actual,
2020
extra_operational_sequencer_classes: extra_operational_sequencer_classes,
2121
extra_diff_formatter_classes: extra_diff_formatter_classes,
22+
sequence: method(:sequence),
2223
)
2324
end
2425

@@ -36,6 +37,7 @@ class LcsCallbacks
3637
:actual!,
3738
:extra_operational_sequencer_classes!,
3839
:extra_diff_formatter_classes!,
40+
:sequence!
3941
],
4042
)
4143
public :operations
@@ -74,7 +76,7 @@ def discard_b(event)
7476
end
7577

7678
def change(event)
77-
child_operations = sequence(event.old_element, event.new_element)
79+
child_operations = sequence.call(event.old_element, event.new_element)
7880

7981
if child_operations
8082
add_change_operation(event, child_operations)
@@ -86,16 +88,6 @@ def change(event)
8688

8789
private
8890

89-
def sequence(expected, actual)
90-
OperationalSequencer.call(
91-
expected: expected,
92-
actual: actual,
93-
all_or_nothing: false,
94-
extra_classes: extra_operational_sequencer_classes,
95-
extra_diff_formatter_classes: extra_diff_formatter_classes,
96-
)
97-
end
98-
9991
def add_change_operation(event, child_operations)
10092
operations << ::SuperDiff::Operations::BinaryOperation.new(
10193
name: :change,

lib/super_diff/operational_sequencers/base.rb

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,9 @@ def build_operation_sequencer
6262

6363
def possible_comparison_of(operation, next_operation)
6464
if should_compare?(operation, next_operation)
65-
sequence(operation, next_operation)
65+
sequence(operation.value, next_operation.value)
66+
else
67+
nil
6668
end
6769
end
6870

@@ -73,10 +75,10 @@ def should_compare?(operation, next_operation)
7375
next_operation.index == operation.index
7476
end
7577

76-
def sequence(operation, next_operation)
78+
def sequence(expected, actual)
7779
OperationalSequencer.call(
78-
expected: operation.value,
79-
actual: next_operation.value,
80+
expected: expected,
81+
actual: actual,
8082
all_or_nothing: false,
8183
extra_classes: extra_operational_sequencer_classes,
8284
extra_diff_formatter_classes: extra_diff_formatter_classes,

lib/super_diff/recursion_guard.rb

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
module SuperDiff
2+
module RecursionGuard
3+
RECURSION_GUARD_KEY = "super_diff_recursion_guard_key".freeze
4+
PLACEHOLDER = "∙∙∙".freeze
5+
6+
def self.guarding_recursion_of(*objects, &block)
7+
already_seen_objects, first_seen_objects = objects.partition do |object|
8+
already_seen_object_ids.include?(object.object_id)
9+
end
10+
11+
first_seen_objects.each do |object|
12+
already_seen_object_ids.add(object.object_id)
13+
end
14+
15+
result =
16+
if block.arity > 0
17+
block.call(already_seen_objects.any?)
18+
else
19+
block.call
20+
end
21+
22+
first_seen_objects.each do |object|
23+
already_seen_object_ids.delete(object.object_id)
24+
end
25+
26+
result
27+
end
28+
29+
def self.substituting_recursion_of(*objects)
30+
guarding_recursion_of(*objects) do |already_seen|
31+
if already_seen
32+
PLACEHOLDER
33+
else
34+
yield
35+
end
36+
end
37+
end
38+
39+
def self.already_seen_objects
40+
already_seen_object_ids.map { |object_id| ObjectSpace._id2ref(object_id) }
41+
end
42+
43+
def self.already_seen_object_ids
44+
Thread.current[RECURSION_GUARD_KEY] ||= Set.new
45+
end
46+
end
47+
end

0 commit comments

Comments
 (0)