Skip to content

Commit b755a2b

Browse files
committed
Fix inspection of empty values in diffs
When inspecting an empty value, such an array, hash, or object — even when using `singleline: false` — don't try to split that value across multiple lines.
1 parent c356609 commit b755a2b

File tree

11 files changed

+337
-34
lines changed

11 files changed

+337
-34
lines changed

lib/super_diff/object_inspection/inspection_tree.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,14 @@ def nested(&block)
5353
add_node :nesting, &block
5454
end
5555

56+
def when_empty(&block)
57+
add_node :when_empty, &block
58+
end
59+
60+
def when_non_empty(&block)
61+
add_node :when_non_empty, &block
62+
end
63+
5664
def insert_array_inspection_of(array)
5765
# FIXME: why must this be inside the `nested`?
5866
add_break

lib/super_diff/object_inspection/inspectors/array.rb

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,20 @@ module SuperDiff
22
module ObjectInspection
33
module Inspectors
44
Array = InspectionTree.new do
5-
add_text "["
6-
7-
nested do |array|
8-
insert_array_inspection_of(array)
5+
when_empty do
6+
add_text "[]"
97
end
108

11-
add_break
12-
add_text "]"
9+
when_non_empty do
10+
add_text "["
11+
12+
nested do |array|
13+
insert_array_inspection_of(array)
14+
end
15+
16+
add_break
17+
add_text "]"
18+
end
1319
end
1420
end
1521
end

lib/super_diff/object_inspection/inspectors/default_object.rb

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,36 +2,44 @@ module SuperDiff
22
module ObjectInspection
33
module Inspectors
44
DefaultObject = InspectionTree.new do
5-
when_singleline do
5+
when_empty do
66
add_text do |object|
77
object.inspect
88
end
99
end
1010

11-
when_multiline do
12-
add_text do |object|
13-
"#<%<class>s:0x%<id>x {" % {
14-
class: object.class,
15-
id: object.object_id * 2,
16-
}
11+
when_non_empty do
12+
when_singleline do
13+
add_text do |object|
14+
object.inspect
15+
end
1716
end
1817

19-
nested do |object|
20-
add_break " "
18+
when_multiline do
19+
add_text do |object|
20+
"#<%<class>s:0x%<id>x {" % {
21+
class: object.class,
22+
id: object.object_id * 2,
23+
}
24+
end
25+
26+
nested do |object|
27+
add_break " "
2128

22-
insert_separated_list(
23-
object.instance_variables.sort,
24-
separator: ","
25-
) do |name|
26-
add_text name.to_s
27-
add_text "="
28-
add_inspection_of object.instance_variable_get(name)
29+
insert_separated_list(
30+
object.instance_variables.sort,
31+
separator: ","
32+
) do |name|
33+
add_text name.to_s
34+
add_text "="
35+
add_inspection_of object.instance_variable_get(name)
36+
end
2937
end
30-
end
3138

32-
add_break
39+
add_break
3340

34-
add_text "}>"
41+
add_text "}>"
42+
end
3543
end
3644
end
3745
end

lib/super_diff/object_inspection/inspectors/hash.rb

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,20 @@ module SuperDiff
22
module ObjectInspection
33
module Inspectors
44
Hash = InspectionTree.new do
5-
add_text "{"
6-
7-
nested do |hash|
8-
insert_hash_inspection_of(hash)
5+
when_empty do
6+
add_text "{}"
97
end
108

11-
add_break " "
12-
add_text "}"
9+
when_non_empty do
10+
add_text "{"
11+
12+
nested do |hash|
13+
insert_hash_inspection_of(hash)
14+
end
15+
16+
add_break " "
17+
add_text "}"
18+
end
1319
end
1420
end
1521
end

lib/super_diff/object_inspection/map.rb

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,9 @@ def call(object)
1616
Inspectors::Hash
1717
when String
1818
Inspectors::String
19-
when true, false, nil, Symbol, Numeric, Regexp
20-
Inspectors::Primitive else
19+
when true, false, nil, Symbol, Numeric, Regexp, Class
20+
Inspectors::Primitive
21+
else
2122
Inspectors::DefaultObject
2223
end
2324
end

lib/super_diff/object_inspection/nodes.rb

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,29 @@ module Nodes
66
autoload :Inspection, "super_diff/object_inspection/nodes/inspection"
77
autoload :Nesting, "super_diff/object_inspection/nodes/nesting"
88
autoload :Text, "super_diff/object_inspection/nodes/text"
9+
autoload(
10+
:WhenEmpty,
11+
"super_diff/object_inspection/nodes/when_empty",
12+
)
913
autoload(
1014
:WhenMultiline,
1115
"super_diff/object_inspection/nodes/when_multiline",
1216
)
17+
autoload(
18+
:WhenNonEmpty,
19+
"super_diff/object_inspection/nodes/when_non_empty",
20+
)
1321
autoload(
1422
:WhenSingleline,
1523
"super_diff/object_inspection/nodes/when_singleline",
1624
)
1725

1826
def self.fetch(type)
1927
registry.fetch(type) do
20-
raise KeyError, "Could not find a node class for #{type.inspect}!"
28+
raise(
29+
KeyError,
30+
"#{type.inspect} is not included in ObjectInspection::Nodes.registry!",
31+
)
2132
end
2233
end
2334

@@ -27,7 +38,9 @@ def self.registry
2738
inspection: Inspection,
2839
nesting: Nesting,
2940
text: Text,
41+
when_empty: WhenEmpty,
3042
when_multiline: WhenMultiline,
43+
when_non_empty: WhenNonEmpty,
3144
when_singleline: WhenSingleline,
3245
}
3346
end
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
module SuperDiff
2+
module ObjectInspection
3+
module Nodes
4+
class WhenEmpty < Base
5+
def evaluate(object, indent_level:, as_single_line:)
6+
if empty?(object)
7+
evaluate_in_subtree(
8+
object,
9+
indent_level: indent_level,
10+
as_single_line: as_single_line,
11+
&block
12+
)
13+
else
14+
""
15+
end
16+
end
17+
18+
private
19+
20+
def empty?(object)
21+
if object.respond_to?(:empty?)
22+
object.empty?
23+
else
24+
object.instance_variables.empty?
25+
end
26+
end
27+
end
28+
end
29+
end
30+
end
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
module SuperDiff
2+
module ObjectInspection
3+
module Nodes
4+
class WhenNonEmpty < Base
5+
def evaluate(object, indent_level:, as_single_line:)
6+
if empty?(object)
7+
""
8+
else
9+
evaluate_in_subtree(
10+
object,
11+
indent_level: indent_level,
12+
as_single_line: as_single_line,
13+
&block
14+
)
15+
end
16+
end
17+
18+
private
19+
20+
def empty?(object)
21+
if object.respond_to?(:empty?)
22+
object.empty?
23+
else
24+
object.instance_variables.empty?
25+
end
26+
end
27+
end
28+
end
29+
end
30+
end

spec/integration/rspec/eq_matcher_spec.rb

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -756,4 +756,119 @@
756756
end
757757
end
758758
end
759+
760+
context "when comparing two data structures where one contains an empty array" do
761+
it "formats the array correctly in the diff" do
762+
as_both_colored_and_uncolored do |color_enabled|
763+
snippet = <<~TEST.strip
764+
expected = { foo: nil }
765+
actual = { foo: [] }
766+
expect(actual).to eq(expected)
767+
TEST
768+
program = make_plain_test_program(snippet, color_enabled: color_enabled)
769+
770+
expected_output = build_expected_output(
771+
color_enabled: color_enabled,
772+
snippet: %|expect(actual).to eq(expected)|,
773+
newline_before_expectation: true,
774+
expectation: proc {
775+
line do
776+
plain "Expected "
777+
green %|{ foo: [] }|
778+
plain " to eq "
779+
red %|{ foo: nil }|
780+
plain "."
781+
end
782+
},
783+
diff: proc {
784+
plain_line %| {|
785+
red_line %|- foo: nil|
786+
green_line %|+ foo: []|
787+
plain_line %| }|
788+
}
789+
)
790+
791+
expect(program).
792+
to produce_output_when_run(expected_output).
793+
in_color(color_enabled)
794+
end
795+
end
796+
end
797+
798+
context "when comparing two data structures where one contains an empty hash" do
799+
it "formats the hash correctly in the diff" do
800+
as_both_colored_and_uncolored do |color_enabled|
801+
snippet = <<~TEST.strip
802+
expected = { foo: nil }
803+
actual = { foo: {} }
804+
expect(actual).to eq(expected)
805+
TEST
806+
program = make_plain_test_program(snippet, color_enabled: color_enabled)
807+
808+
expected_output = build_expected_output(
809+
color_enabled: color_enabled,
810+
snippet: %|expect(actual).to eq(expected)|,
811+
newline_before_expectation: true,
812+
expectation: proc {
813+
line do
814+
plain "Expected "
815+
green %|{ foo: {} }|
816+
plain " to eq "
817+
red %|{ foo: nil }|
818+
plain "."
819+
end
820+
},
821+
diff: proc {
822+
plain_line %| {|
823+
red_line %|- foo: nil|
824+
green_line %|+ foo: {}|
825+
plain_line %| }|
826+
}
827+
)
828+
829+
expect(program).
830+
to produce_output_when_run(expected_output).
831+
in_color(color_enabled)
832+
end
833+
end
834+
end
835+
836+
context "when comparing two data structures where one contains an empty object" do
837+
it "formats the object correctly in the diff" do
838+
as_both_colored_and_uncolored do |color_enabled|
839+
snippet = <<~TEST.strip
840+
expected = { foo: nil }
841+
actual = { foo: SuperDiff::Test::EmptyClass.new }
842+
expect(actual).to eq(expected)
843+
TEST
844+
program = make_plain_test_program(snippet, color_enabled: color_enabled)
845+
846+
expected_output = build_expected_output(
847+
color_enabled: color_enabled,
848+
snippet: %|expect(actual).to eq(expected)|,
849+
newline_before_expectation: true,
850+
expectation: proc {
851+
line do
852+
plain "Expected "
853+
green %|{ foo: #<SuperDiff::Test::EmptyClass> }|
854+
plain " to eq "
855+
red %|{ foo: nil }|
856+
plain "."
857+
end
858+
},
859+
diff: proc {
860+
plain_line %| {|
861+
red_line %|- foo: nil|
862+
green_line %|+ foo: #<SuperDiff::Test::EmptyClass>|
863+
plain_line %| }|
864+
}
865+
)
866+
867+
expect(program).
868+
to produce_output_when_run(expected_output).
869+
in_color(color_enabled).
870+
removing_object_ids
871+
end
872+
end
873+
end
759874
end

spec/support/models/empty_class.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
module SuperDiff
2+
module Test
3+
class EmptyClass
4+
end
5+
end
6+
end

0 commit comments

Comments
 (0)