Skip to content

Commit 90ddfbe

Browse files
wa_diff: tagged-union diff()/side() for stricter eqwalizer
Summary: The upcoming eqwalizer treats `tuple()` strictly and rejects `wa_diff`s current representation, where raw `supported_input()` values are stored directly in `side()`-typed positions. This type checks today only because the current eqwalizer handles `tuple()` loosely; making the representation a set of properly tagged unions fixes it ahead of the upgrade. `diff()` is now `{eq, supported_input()} | {diff, side(), side()}` instead of a `#{equivalent, left, right}` map: when two values are equivalent the raw input stays in its own `eq` arm and never flows into `side()`. The leaf arms of `side()` are tagged: the untagged `literal()` member becomes `{literal, literal()}`, and a new `value_to_side/1` wraps raw values as `{literal, _}` for scalars or reuses `{eq, _}` for compound terms that are rendered whole. To keep this consistent `block()` now holds `[side()]` and the struct arm carries `[struct_arg()]`, the `{Key, ValueSide}` pairs and whole-item blocks that maps and records actually store. Rendered output is unchanged: every leaf is still emitted via whole-value `inspect`. Reviewed By: robertoaloi Differential Revision: D108615566 fbshipit-source-id: d939f2b7257847b915c2ca90484afbcb35177832
1 parent edf0e89 commit 90ddfbe

3 files changed

Lines changed: 99 additions & 110 deletions

File tree

‎README.md‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -75,15 +75,14 @@ powered by the [diffy](https://github.com/zotonic/diffy) implementation of
7575
[Neil Fraser's match-and-patch algorithm](https://neil.fraser.name/writing/diff/),
7676
it provides a way to diff Erlang data types such as maps, records, lists, tuples and more.
7777

78-
The main entry point is the `compute/2` function which, given two values, produces *left* and *right* parts:
78+
The main entry point is the `compute/2` function which, given two values,
79+
produces either `{eq, Value}` when the two values are equivalent, or a
80+
`{diff, Left, Right}` tuple holding the *left* and *right* parts:
7981

8082
```erlang
81-
> #{left := Left, right := Right, equivalent := Equivalent} = wa_diff:compute(#{one => 1, two => 2}, #{one => 1, two => 3}).
82-
#{left =>
83-
{map,[],[{one,1},{two,#{contents => [{true,<<"2">>}]}}]},
84-
right =>
85-
{map,[],[{one,1},{two,#{contents => [{true,<<"3">>}]}}]},
86-
equivalent => false}
83+
> {diff, Left, Right} = wa_diff:compute(#{one => 1, two => 2}, #{one => 1, two => 3}).
84+
{diff,{map,[],[{one,{eq,1}},{two,#{contents => [{true,<<"2">>}]}}]},
85+
{map,[],[{one,{eq,1}},{two,#{contents => [{true,<<"3">>}]}}]}}
8786
```
8887

8988
Each part can then be rendered via the `to_diff/3` function. For example:

‎src/wa_diff.erl‎

Lines changed: 71 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@
2424
error_info/3,
2525
compute/2, compute/3,
2626
to_algebra/2,
27-
to_diff/3
27+
to_diff/3,
28+
to_sides/1
2829
]).
2930

3031
-export([red/0, green/0, yellow/0, reset/0]).
@@ -33,20 +34,24 @@
3334

3435
-export([glue/3, join_docs/2]).
3536

36-
-export_type([supported_input/0]).
37+
-export_type([supported_input/0, diff/0, side/0]).
3738

38-
-type diff() :: #{equivalent := boolean(), left := side(), right := side()}.
39+
-type diff() :: {eq, supported_input()} | {diff, side(), side()}.
3940
-type side() ::
4041
contents()
41-
| literal()
42+
| {literal, literal()}
4243
| {container_type(), meta(), [side()]}
4344
| block()
44-
| {struct_type(), meta(), [side()]}
45-
| struct_item().
45+
| {struct_type(), meta(), [struct_arg()]}
46+
| struct_item()
47+
| {eq, supported_input()}.
48+
%% A struct (map/record) carries either a `{Key, ValueSide}` pair for keys
49+
%% present on both sides, or a whole-item `block()` for keys present on one side.
50+
-type struct_arg() :: {supported_input(), side()} | block().
4651
-type contents() :: #{contents := [content()]}.
4752
-type content() :: {boolean(), binary()}.
4853
-type literal() :: number() | atom() | binary() | string().
49-
-type block() :: {'__block__', meta(), [supported_input()]}.
54+
-type block() :: {'__block__', meta(), [side()]}.
5055
-type meta() :: [{diff, boolean()}].
5156
-type script() :: diffy:diffs().
5257
-type doc() :: erlfmt_algebra:doc().
@@ -72,9 +77,9 @@ error_info(Module, Left0, Right0) ->
7277
Ref = make_ref(),
7378
spawn(
7479
fun() ->
75-
Diff = compute(Left0, Right0, Module),
76-
{ok, Left} = to_diff(maps:get(left, Diff), red(), reset()),
77-
{ok, Right} = to_diff(maps:get(right, Diff), green(), reset()),
80+
{LeftSide, RightSide} = to_sides(compute(Left0, Right0, Module)),
81+
{ok, Left} = to_diff(LeftSide, red(), reset()),
82+
{ok, Right} = to_diff(RightSide, green(), reset()),
7883
Cause = #{left => Left, right => Right},
7984
Self ! {Ref, #{module => ?MODULE, function => format_error, cause => Cause}}
8085
end
@@ -149,6 +154,12 @@ compute(Left, Right, Module) ->
149154
to_algebra(Side, DiffWrapper) ->
150155
wrap_on_diff(Side, fun safe_to_algebra/2, DiffWrapper).
151156

157+
-spec to_sides(diff()) -> {side(), side()}.
158+
to_sides({eq, Same}) ->
159+
{{eq, Same}, {eq, Same}};
160+
to_sides({diff, Left, Right}) ->
161+
{Left, Right}.
162+
152163
-spec format_error(term(), erlang:stacktrace()) -> error_description().
153164
format_error(Reason, [{_M, _F, _Args, Info} | _]) ->
154165
ErrorInfo = proplists:get_value(error_info, Info, #{}),
@@ -162,7 +173,7 @@ format_error(Reason, [{_M, _F, _Args, Info} | _]) ->
162173

163174
-spec diff_value(supported_input(), supported_input(), context()) -> diff().
164175
diff_value(Same, Same, _Context) ->
165-
#{equivalent => true, left => Same, right => Same};
176+
{eq, Same};
166177
diff_value(Left, Right, _Context) when is_number(Left), is_number(Right) ->
167178
diff_literal(Left, Right);
168179
diff_value(Left, Right, _Context) when is_atom(Left), is_atom(Right) ->
@@ -199,9 +210,9 @@ is_printable(Binary) ->
199210

200211
-spec non_recursive_diff_value(supported_input(), supported_input()) -> diff().
201212
non_recursive_diff_value(Left0, Right0) ->
202-
Left = update_diff_meta(Left0, true),
203-
Right = update_diff_meta(Right0, true),
204-
#{equivalent => false, left => Left, right => Right}.
213+
Left = update_diff_meta(value_to_side(Left0), true),
214+
Right = update_diff_meta(value_to_side(Right0), true),
215+
{diff, Left, Right}.
205216

206217
-spec diff_literal(literal(), literal()) -> diff().
207218
diff_literal(Left, Right) ->
@@ -210,42 +221,36 @@ diff_literal(Left, Right) ->
210221
-spec diff_string(binary(), binary()) -> diff().
211222
diff_string(Left, Right) ->
212223
Script = diffy:diff(Left, Right),
213-
string_script_to_diff(Script, true, [], []).
224+
string_script_to_diff(Script, [], []).
214225

215226
-spec diff_container(container_type(), [supported_input()], [supported_input()], context()) ->
216227
diff().
217228
diff_container(Type, Left, Right, Context) ->
218-
diff_container(Type, Left, Right, true, [], [], Context).
229+
diff_container(Type, Left, Right, [], [], Context).
219230

220231
-spec diff_container(
221232
container_type(),
222233
[supported_input()],
223234
[supported_input()],
224-
boolean(),
225235
[side()],
226236
[side()],
227237
context()
228238
) ->
229239
diff().
230-
diff_container(Type, [HLeft | TLeft], [HRight | TRight], Equivalent0, AccLeft, AccRight, Context) ->
231-
#{equivalent := Equivalent, left := Left, right := Right} = diff_value(HLeft, HRight, Context),
240+
diff_container(Type, [HLeft | TLeft], [HRight | TRight], AccLeft, AccRight, Context) ->
241+
{Left, Right} = to_sides(diff_value(HLeft, HRight, Context)),
232242
diff_container(
233243
Type,
234244
TLeft,
235245
TRight,
236-
Equivalent0 andalso Equivalent,
237246
[Left | AccLeft],
238247
[Right | AccRight],
239248
Context
240249
);
241-
diff_container(Type, Left0, Right0, Equivalent, AccLeft, AccRight, _Context) ->
242-
Left = [update_diff_meta(L, true) || L <- Left0],
243-
Right = [update_diff_meta(L, true) || L <- Right0],
244-
#{
245-
equivalent => Equivalent andalso Left =:= [] andalso Right =:= [],
246-
left => {Type, [], lists:reverse(AccLeft, Left)},
247-
right => {Type, [], lists:reverse(AccRight, Right)}
248-
}.
250+
diff_container(Type, Left0, Right0, AccLeft, AccRight, _Context) ->
251+
Left = [update_diff_meta(value_to_side(L), true) || L <- Left0],
252+
Right = [update_diff_meta(value_to_side(L), true) || L <- Right0],
253+
{diff, {Type, [], lists:reverse(AccLeft, Left)}, {Type, [], lists:reverse(AccRight, Right)}}.
249254

250255
-spec diff_struct(
251256
struct_type(),
@@ -255,38 +260,40 @@ diff_container(Type, Left0, Right0, Equivalent, AccLeft, AccRight, _Context) ->
255260
) -> diff().
256261
diff_struct(Type, Left0, Right0, Context) ->
257262
Acc0 = {
258-
_Equivalent = true,
259263
_AccLeft = [],
260264
_AccRight = [],
261265
_PendingLeft = [],
262266
_PendingRight = Right0
263267
},
264-
Fun = fun(LeftKey, LeftValue, {Equivalent0, AccLeft, AccRight, PendingLeft0, PendingRight0}) ->
268+
Fun = fun(LeftKey, LeftValue, {AccLeft, AccRight, PendingLeft0, PendingRight0}) ->
265269
case maps:take(LeftKey, PendingRight0) of
266270
error ->
267271
PendingLeft = [
268-
update_diff_meta({struct_item, LeftKey, divider(Type), LeftValue}, true)
272+
update_diff_meta(
273+
{struct_item, value_to_side(LeftKey), divider(Type), value_to_side(LeftValue)},
274+
true
275+
)
269276
| PendingLeft0
270277
],
271-
{false, AccLeft, AccRight, PendingLeft, PendingRight0};
278+
{AccLeft, AccRight, PendingLeft, PendingRight0};
272279
{RightValue, PendingRight} ->
273-
Diff = diff_value(LeftValue, RightValue, Context),
274-
#{equivalent := Equivalent, left := Left, right := Right} = Diff,
280+
{Left, Right} = to_sides(diff_value(LeftValue, RightValue, Context)),
275281
{
276-
Equivalent0 andalso Equivalent,
277282
[{LeftKey, Left} | AccLeft],
278283
[{LeftKey, Right} | AccRight],
279284
PendingLeft0,
280285
PendingRight
281286
}
282287
end
283288
end,
284-
{AccEquivalent, AccLeft, AccRight, PendingLeft, PendingRight0} = maps:fold(Fun, Acc0, Left0),
285-
PendingRight = [update_diff_meta({struct_item, K, divider(Type), V}, true) || K := V <- PendingRight0],
286-
Equivalent = AccEquivalent andalso (maps:size(PendingRight0) =:= 0),
289+
{AccLeft, AccRight, PendingLeft, PendingRight0} = maps:fold(Fun, Acc0, Left0),
290+
PendingRight = [
291+
update_diff_meta({struct_item, value_to_side(K), divider(Type), value_to_side(V)}, true)
292+
|| K := V <- PendingRight0
293+
],
287294
Left = lists:sort(AccLeft) ++ lists:sort(PendingLeft),
288295
Right = lists:sort(AccRight) ++ lists:sort(PendingRight),
289-
#{equivalent => Equivalent, left => {Type, [], Left}, right => {Type, [], Right}}.
296+
{diff, {Type, [], Left}, {Type, [], Right}}.
290297

291298
-spec divider(struct_type()) -> delimiter().
292299
divider(map) -> <<" => ">>;
@@ -313,10 +320,18 @@ expand_tuple([Name | Values], #{records := Records} = _Context) when is_atom(Nam
313320
expand_tuple(_, _) ->
314321
not_found.
315322

316-
-spec update_diff_meta(supported_input(), boolean()) -> block().
323+
-spec update_diff_meta(side(), boolean()) -> block().
317324
update_diff_meta(Value, true) ->
318325
{'__block__', [{diff, true}], [Value]}.
319326

327+
%% Wrap a raw value as a leaf side(): scalars become tagged literals,
328+
%% compound terms (tuples, maps) are rendered whole via the `eq` tag.
329+
-spec value_to_side(supported_input()) -> side().
330+
value_to_side(Value) when is_number(Value); is_atom(Value); is_binary(Value) ->
331+
{literal, Value};
332+
value_to_side(Value) ->
333+
{eq, Value}.
334+
320335
-spec inspect(term()) -> binary().
321336
inspect(Term) ->
322337
inspect(Term, "~0tp").
@@ -357,14 +372,16 @@ safe_to_algebra(#{contents := Contents}, DiffWrapper) ->
357372
DiffWrapper(Content)
358373
end,
359374
erlfmt_algebra:concat([Fun(Elem) || Elem <- Contents]);
360-
safe_to_algebra(Literal, _DiffWrapper) ->
375+
safe_to_algebra({eq, Value}, _DiffWrapper) ->
376+
erlfmt_algebra:string(inspect(Value));
377+
safe_to_algebra({literal, Literal}, _DiffWrapper) ->
361378
erlfmt_algebra:string(inspect(Literal)).
362379

363-
-spec with_struct_divider(side() | {side(), side()}, delimiter()) -> side().
380+
-spec with_struct_divider(block() | {supported_input(), side()}, delimiter()) -> side().
364381
with_struct_divider({K, V}, Divider) ->
365-
{struct_item, K, Divider, V};
366-
with_struct_divider(Side, _Divider) ->
367-
Side.
382+
{struct_item, value_to_side(K), Divider, V};
383+
with_struct_divider(Block, _Divider) ->
384+
Block.
368385

369386
-spec container_to_algebra(delimiter(), [side()], delimiter(), wrapper_fun(), fun((side(), wrapper_fun()) -> doc())) ->
370387
doc().
@@ -406,20 +423,16 @@ wrap_on_diff(Side, Fun, DiffWrapper) ->
406423
extract_diff_meta({'__block__', [{diff, true}], [Literal]}) -> {Literal, true};
407424
extract_diff_meta(Other) -> {Other, false}.
408425

409-
-spec string_script_to_diff(script(), boolean(), [{boolean(), binary()}], [{boolean(), binary()}]) ->
426+
-spec string_script_to_diff(script(), [{boolean(), binary()}], [{boolean(), binary()}]) ->
410427
diff().
411-
string_script_to_diff([], Equivalent, Left, Right) ->
412-
#{
413-
equivalent => Equivalent,
414-
left => #{contents => lists:reverse(Left)},
415-
right => #{contents => lists:reverse(Right)}
416-
};
417-
string_script_to_diff([{equal, String} | Tail], Equivalent, Left0, Right0) ->
418-
string_script_to_diff(Tail, Equivalent, [{false, String} | Left0], [{false, String} | Right0]);
419-
string_script_to_diff([{delete, String} | Tail], _Equivalent, Left0, Right0) ->
420-
string_script_to_diff(Tail, false, [{true, String} | Left0], Right0);
421-
string_script_to_diff([{insert, String} | Tail], _Equivalent, Left0, Right0) ->
422-
string_script_to_diff(Tail, false, Left0, [{true, String} | Right0]).
428+
string_script_to_diff([], Left, Right) ->
429+
{diff, #{contents => lists:reverse(Left)}, #{contents => lists:reverse(Right)}};
430+
string_script_to_diff([{equal, String} | Tail], Left0, Right0) ->
431+
string_script_to_diff(Tail, [{false, String} | Left0], [{false, String} | Right0]);
432+
string_script_to_diff([{delete, String} | Tail], Left0, Right0) ->
433+
string_script_to_diff(Tail, [{true, String} | Left0], Right0);
434+
string_script_to_diff([{insert, String} | Tail], Left0, Right0) ->
435+
string_script_to_diff(Tail, Left0, [{true, String} | Right0]).
423436

424437
-spec to_diff(side(), doc(), doc()) -> {ok, string()} | {error, term()}.
425438
to_diff(Side, Pre, Post) ->

‎test/wa_diff_SUITE.erl‎

Lines changed: 22 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -46,57 +46,38 @@ all() ->
4646

4747
compute(_Config) ->
4848
?assertEqual(
49-
#{
50-
left => #{contents => [{true, <<"1">>}]},
51-
right => #{contents => [{true, <<"2">>}]},
52-
equivalent => false
53-
},
49+
{diff, #{contents => [{true, <<"1">>}]}, #{contents => [{true, <<"2">>}]}},
5450
wa_diff:compute(1, 2)
5551
),
5652
?assertEqual(
57-
#{
58-
left => #{contents => [{false, <<"1">>}]},
59-
right => #{contents => [{true, <<"2">>}, {false, <<"1">>}]},
60-
equivalent => false
61-
},
53+
{diff, #{contents => [{false, <<"1">>}]}, #{contents => [{true, <<"2">>}, {false, <<"1">>}]}},
6254
wa_diff:compute(1, 21)
6355
),
6456
?assertEqual(
65-
#{
66-
left => {1},
67-
right => {1},
68-
equivalent => true
69-
},
57+
{eq, {1}},
7058
wa_diff:compute({1}, {1})
7159
),
7260
?assertEqual(
73-
#{
74-
left => {tuple, [], [#{contents => [{true, <<"1">>}]}]},
75-
right => {tuple, [], [#{contents => [{true, <<"2">>}]}]},
76-
equivalent => false
77-
},
61+
{diff, {tuple, [], [#{contents => [{true, <<"1">>}]}]}, {tuple, [], [#{contents => [{true, <<"2">>}]}]}},
7862
wa_diff:compute({1}, {2})
7963
),
8064
?assertEqual(
81-
#{
82-
left => {tuple, [], [1]},
83-
right =>
84-
{tuple, [], [
85-
1, {'__block__', [{diff, true}], [2]}, {'__block__', [{diff, true}], [3]}
86-
]},
87-
equivalent => false
88-
},
65+
{diff, {tuple, [], [{eq, 1}]},
66+
{tuple, [], [
67+
{eq, 1},
68+
{'__block__', [{diff, true}], [{literal, 2}]},
69+
{'__block__', [{diff, true}], [{literal, 3}]}
70+
]}},
8971
wa_diff:compute({1}, {1, 2, 3})
9072
),
9173
?assertEqual(
92-
#{
93-
left => {map, [], [{a, 1}]},
94-
right =>
95-
{map, [], [
96-
{a, 1}, {'__block__', [{diff, true}], [{struct_item, b, <<" => ">>, 2}]}
97-
]},
98-
equivalent => false
99-
},
74+
{diff, {map, [], [{a, {eq, 1}}]},
75+
{map, [], [
76+
{a, {eq, 1}},
77+
{'__block__', [{diff, true}], [
78+
{struct_item, {literal, b}, <<" => ">>, {literal, 2}}
79+
]}
80+
]}},
10081
wa_diff:compute(#{a => 1}, #{a => 1, b => 2})
10182
).
10283

@@ -350,18 +331,14 @@ assertions(_Config) ->
350331
%% Internal Helpers
351332
%%--------------------------------------------------------------------
352333
assert_diff(Left, Right) ->
353-
Diff = wa_diff:compute(Left, Right, ?MODULE),
354-
?assert(maps:get(equivalent, Diff)).
334+
?assertMatch({eq, _}, wa_diff:compute(Left, Right, ?MODULE)).
355335

356336
refute_diff(Left, Right, ExpectedLeft, ExpectedRight) ->
357337
Diff = wa_diff:compute(Left, Right, ?MODULE),
358-
?assertNot(maps:get(equivalent, Diff)),
359-
{ok, DiffLeft} = wa_diff:to_diff(
360-
maps:get(left, Diff), ~"-", ~"-"
361-
),
362-
{ok, DiffRight} = wa_diff:to_diff(
363-
maps:get(right, Diff), ~"+", ~"+"
364-
),
338+
?assertMatch({diff, _, _}, Diff),
339+
{diff, LeftSide, RightSide} = Diff,
340+
{ok, DiffLeft} = wa_diff:to_diff(LeftSide, ~"-", ~"-"),
341+
{ok, DiffRight} = wa_diff:to_diff(RightSide, ~"+", ~"+"),
365342
?assertEqual(ExpectedLeft, DiffLeft),
366343
?assertEqual(ExpectedRight, DiffRight).
367344

0 commit comments

Comments
 (0)